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

Add a PathHandler implementation for Azure blob. #17

Closed
wants to merge 10 commits into from

Conversation

davides
Copy link
Contributor

@davides davides commented Oct 19, 2022

Notes

Adds a new PathHandler to support read/write with Azure Blob Storage. Some controls have been put in place so that read/write operations use a known amount of memory when dealing with larger files:

  • _open("wb", buffering=<buffer-size>) will buffer up to the requested amount of data in memory before flushing it to the service with the PutBlock operation
  • _open("rb", buffering=<buffer-size>) will use the Blob client's chunk iterator to only download a fixed amount of data at a time
  • _close() in write-mode will flush any buffered data with one more PutBlock, and finalize the blob with PutBlockList

The block-based approach should work for both block blobs and append blobs (see the Azure docs).

Testing

$ python -m tests.test_azure_blob
......
----------------------------------------------------------------------
Ran 6 tests in 1.992s

OK

@davides davides requested a review from sujitoc October 19, 2022 22:58
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 19, 2022
* Add support for downloading directories
Copy link
Contributor

@sujitoc sujitoc left a comment

Choose a reason for hiding this comment

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

Can you move .gitignore to a separate PR with just that change?

iopath/common/azure_blob.py Outdated Show resolved Hide resolved
iopath/common/azure_blob.py Show resolved Hide resolved
iopath/common/azure_blob.py Show resolved Hide resolved
iopath/common/azure_blob.py Show resolved Hide resolved
iopath/common/azure_blob.py Show resolved Hide resolved
iopath/common/azure_blob.py Show resolved Hide resolved
tests/test_azure_blob.py Outdated Show resolved Hide resolved
iopath/common/azure_blob.py Show resolved Hide resolved
@sujitoc sujitoc self-requested a review November 23, 2022 18:41
Copy link
Contributor

@sujitoc sujitoc left a comment

Choose a reason for hiding this comment

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

Discussed with @davides and we decided the remaining issue(s) can be addressed in a follow up.

@facebook-github-bot
Copy link
Contributor

@davides has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants