Skip to content

Conversation

@benjaminkaplan
Copy link

Refactor metadata retrieval into a separate method. No reason to fetch metadata synchronously when creating the AzureBlobFile object. Still enables metadata fetching using the new get_metadata method.

This saves about ~50ms per file open.

Generally synchronous calls are avoided without the user specifically requesting them. Therefore there is no reason to perform a sync call every time an object is created.

Refactor metadata retrieval into a separate method. 
No reason to fetch metadata synchronously when creating the AzureBlobFile object. 
Still enables metadata fetching using the new get_metadata method
@benjaminkaplan
Copy link
Author

I submitted a fix for a typo in my previous commit on this PR. Can you please run the tests again

@benjaminkaplan
Copy link
Author

I see the tests are failing but with no explanation. It just says Process finished with exit code 1

Is there a way to see why this is failing?

@anjaliratnam-msft
Copy link
Collaborator

@benjaminkaplan
This makes sense to not fetch the metadata every time for reads, however, if someone were to access the metadata parameter, it would no longer be populated which could cause problems. Instead maybe it would make more sense to lazily load it and only populate the metadata when it is actually accessed? We should also probably add a test to make sure this behavior is as expected. Open to other ideas as well though!

For the failing tests, it's due to linting errors, if you click on the check you should be able to see the exact error. Running pre-commit run --all-files should take care of this!

@nateprewitt
Copy link
Collaborator

Hi @benjaminkaplan, thanks for the contribution! This seems like an easy win to help with performance. I do however agree with @anjaliratnam-msft that introducing a new method will likely cause usability issues.

I made an alternative proposal in #528 that let's us keep backwards compatibility while still realizing the deferred evaluation savings. Let me know if you have any concerns, otherwise we'll plan to move forward with that.

@benjaminkaplan
Copy link
Author

I think your solution is very elegant and we should move forward with it.

I'll go ahead and close this PR

@benjaminkaplan benjaminkaplan deleted the patch-1 branch January 14, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants