Skip to content

Add read method to StorageStreamDownloader#24275

Merged
jalauzon-msft merged 16 commits intoAzure:mainfrom
jalauzon-msft:downloader-read
Aug 9, 2022
Merged

Add read method to StorageStreamDownloader#24275
jalauzon-msft merged 16 commits intoAzure:mainfrom
jalauzon-msft:downloader-read

Conversation

@jalauzon-msft
Copy link
Copy Markdown
Member

@jalauzon-msft jalauzon-msft commented May 3, 2022

This change adds a standard read(size=-1) method to the StorageStreamDownloader class to make it more like a Python stream. This method acts as one would expect, accepting a size to read and keeping track of a read offset internally. For now, the stream is non-seekable and forward reading only but I don't think there is anything preventing us from making this seekable in the future if desired. It will use an existing _ChunkDownloader to download chunks as needed based on the read size.

This change also adds support for async streams (classes with an async read method) to async upload_blob. This was needed to support using StorageStreamDownloader as an input to upload_blob.

@ghost ghost added the Storage Storage Service (Queues, Blobs, Files) label May 3, 2022
@azure-sdk
Copy link
Copy Markdown
Collaborator

API change check for azure-storage-blob

API changes have been detected in azure-storage-blob. You can review API changes here

API changes

+     def read(self, size: Optional[int] = -1) -> Union[bytes, str]
+ 

@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Jul 15, 2022
@ghost
Copy link
Copy Markdown

ghost commented Jul 15, 2022

Hi @jalauzon-msft. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@jalauzon-msft
Copy link
Copy Markdown
Member Author

Please don't close this bot, I'll be getting back to this work soon.

@ghost ghost removed the no-recent-activity There has been no recent activity on this issue. label Jul 15, 2022
@jalauzon-msft jalauzon-msft marked this pull request as ready for review July 19, 2022 21:45
@vincenttran-msft vincenttran-msft changed the title Add read method to SorageStreamDownloader Add read method to StorageStreamDownloader Jul 19, 2022
@azure-sdk
Copy link
Copy Markdown
Collaborator

azure-sdk commented Jul 19, 2022

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-storage-blob
azure-storage-file-share
azure-storage-file-datalake
azure-storage-queue
azure-storage-blob

Copy link
Copy Markdown
Member

@vincenttran-msft vincenttran-msft left a comment

Choose a reason for hiding this comment

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

LGTM! Great work, this method looks quite hairy with that parallel stuff 😨

@jalauzon-msft
Copy link
Copy Markdown
Member Author

/azp run python - storage - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jalauzon-msft jalauzon-msft linked an issue Jul 22, 2022 that may be closed by this pull request
Copy link
Copy Markdown
Member

@annatisch annatisch left a comment

Choose a reason for hiding this comment

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

I think this is looking really good - thanks so much for this!!

@jalauzon-msft
Copy link
Copy Markdown
Member Author

/azp run python - storage - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jalauzon-msft jalauzon-msft merged commit b7d4785 into Azure:main Aug 9, 2022
@jalauzon-msft jalauzon-msft deleted the downloader-read branch August 9, 2022 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storage Stream Downloader

4 participants