Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azure: raise FileNotFoundError when requested blob is missing from storage #1358

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

alxbridge
Copy link

This PR implements behaviour in AzureStorage._open similar to that already found in S3Storage._open, raising a FileNotFoundError if the blob doesn't exist in the storage.

…ling

Azure: raise FileNotFoundError when requested blob is missing from storage
@jschneier
Copy link
Owner

Thanks, makes sense to fix this.

Any idea about doing the same kind of RAII-style handling in init of AzureStorageFile rather than calling .exists()? Might be able to save an API call that way (which costs money).

client_mock.exists.side_effect = [True, False]
self.storage._client.get_blob_client.return_value = client_mock

file_that_exists = self.storage._open('file-that-exists')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this to use open() as that is the public API. Also to use a with context handler.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just pushed commits to make these changes

@alxbridge
Copy link
Author

Thanks, makes sense to fix this.

Any idea about doing the same kind of RAII-style handling in init of AzureStorageFile rather than calling .exists()? Might be able to save an API call that way (which costs money).

I'm not sure I understand the existing code / Azure SDK well enough to say, tbh. It looks like where that's done in the S3File class, it's conditional on the file being opened in read mode, but the init for AzureStorageFile isn't currently passed similar information on the intended mode for the file.

@alxbridge alxbridge force-pushed the fix/azure-missing-blob-handling branch from eadfbf8 to 9be64e6 Compare May 29, 2024 08:46
Copy link
Owner

@jschneier jschneier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break since #1422. My suggestion is to do something similar to S3 and reference self.file in the constructor of AzureFile since that will properly try to download the file.

This version I think also will break if a file is opened for writing.

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.

2 participants