Skip to content

[Storage] Download stream refactor#7848

Merged
annatisch merged 36 commits into
Azure:feature/storage-preview5from
annatisch:storage-downloads
Oct 19, 2019
Merged

[Storage] Download stream refactor#7848
annatisch merged 36 commits into
Azure:feature/storage-preview5from
annatisch:storage-downloads

Conversation

@annatisch

@annatisch annatisch commented Oct 12, 2019

Copy link
Copy Markdown
Member

Refactored the StorageStreamDownloader objects to:

@adxsdk6

adxsdk6 commented Oct 12, 2019

Copy link
Copy Markdown

Can one of the admins verify this patch?

@annatisch annatisch added the Storage Storage Service (Queues, Blobs, Files) label Oct 12, 2019
@annatisch
annatisch marked this pull request as ready for review October 12, 2019 15:39
@annatisch
annatisch requested review from johanste and lmazuel October 12, 2019 15:40
@rakshith91 rakshith91 added the P0 label Oct 12, 2019
@rakshith91

Copy link
Copy Markdown
Contributor

Would be great if you can update History.md :)
Thanks.

@annatisch

Copy link
Copy Markdown
Member Author

@rakshith91 - Done.

Comment thread sdk/storage/azure-storage-blob/azure/storage/blob/__init__.py Outdated
Comment thread sdk/storage/azure-storage-blob/azure/storage/blob/aio/blob_client_async.py Outdated

@rakshith91 rakshith91 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM..thanks

@annatisch
annatisch requested a review from xiafu-msft October 14, 2019 23:53
@mayurid mayurid added the blocking-release Blocks release label Oct 15, 2019
self.download_size = min(self.file_size, self.length - self.offset + 1)
elif self.offset is not None:
self.download_size = self.file_size - self.offset
self._file_size = parse_length_from_content_range(response.properties.content_range)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a question about file_size. If we uploaded a 30 bytes file with client side encryption, then the encrypted file is 32 bytes. When we want to download the file, the parsed self._file_size is 32 I assume, are we intended to keep 32?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The file size here is purely with regards to the bytes being downloaded, and the values used in the range header. So if the encrypted file is 32, that will be the value we use. If this does not match the decrypted value size, we don't update it... I think that's valid, as the size attribute is to determine the size of the Stream (i.e. what's being downloaded).

@annatisch

Copy link
Copy Markdown
Member Author

/azp run python - storage - ci

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@xiafu-msft

Copy link
Copy Markdown
Contributor

It looks great 😀

@annatisch

Copy link
Copy Markdown
Member Author

/azp run python - storage - ci

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@annatisch
annatisch merged commit 6e31ddf into Azure:feature/storage-preview5 Oct 19, 2019
rakshith91 pushed a commit that referenced this pull request Oct 23, 2019
* Fix SubStream to respect IOBase protocol (#7843)

* removes NoRetry policy (#7845)

* [storage] Makes signed_identifiers a required param (#7844)

* makes signed_identifiers a required param

* fix tests and update history

* removes unecessary check

* fix files and queues tests

* [storage] Changes `file_permission_key` param to `permission_key` (#7841)

* changes file_permission_key param to permission_key

* update history

* Change the param directive to keyword in docstrings (#7855)

* Change the directive to keyword in docstrings

* pylint :(

* [storage] Adds from_string to models (#7870)

* adds from_string to models

* fixes to naming and history

* remove _str in docstrings

* pylint

* [storage-file] await async poller (#7872)

* await files async poller

* fix docstrings

* fix type hints

* [File and Queue] Client constructors (#7853)

* from_queue_url

* file client constructors

* update CHANGELOG

* fix test

* lint fix

* some doc changes

* queue docs

* blob docs

* [storage-queue] rename queue messages (#7895)

* rename queue messages

* update history

* [Storage-Queue][Storage-File] Kwargify positional params (#7877)

* Queue kwargification

* kwargify Files

* small fix

* minor fix

* comments + lint

* some changes

* pylint

* [storage-file, queue] moves param protocol to kwargs in gen_shared_access_signature() (#7897)

* moves protocol in gen_sas to kwargs

* delete whitespace

* [storage] Unexposes models from aio (#7881)

* unexposes models from aio

* unexpose all except clients and pylint fixes

* update history

* unexpose sync paged models

* update history phrasing

* [Storage-queue] Allow None message encode policy (#7898)

* Don't expose NoEncode policies

* Support None value for encode/decode policies

* Updated tests

* Updated sync encryption tests

* Updated async encryption tests

* Updated async encoding tests

* Fixed test

* Fix test

* Updated release notes

* Rename enqueue_message to send_messgae (#7928)

* [storage-blob, queue] Renames Logging to <ServiceName>AnalyticsLogging (#7921)

* renames logging to ServiceAnalyticsLogging

* changes param logging to analytics_logging

* fixes tests due to param name change

* [Blob][File][Encryption]Fix Download Encrypted Blob/File Bug (#7965)

#7957

* Consolidate offset - length behavior in files (#7942)

* Initital Commit

* length changes

* some test changes

* some more changes

* history.md

* Update a couple of recrodings

* comments address

* ops

* Stop modifying internal_response body (#7958)

* Stop modifying internal body

* Replicate for queues and files too

* [Storage] Relocated SAS generation (#7955)

* Expose account name on clients

* Updated blob sas gen

* Updated file sas gen

* Update queue sas gen

* Updated docstrings

* Some pylint fixes

* More pylint

* More pylint

* Reverted auth change

* Added release notes

* Fixed kwarg docstring

* [Storage] Updated close handles (#7940)

* Updated close handles

* Pylint fixes

* Fix tests

* Review feedback

* Missing recursive parameter

* Added release notes

* [Storage] Conditional etag parameters (#8047)

* Updated clients

* Process conditional headers

* Updated tests

* Fixed test

* Updated error scenarios

* Updated release notes

* Revert skipped tests

* [Storage] Download stream refactor (#7848)

* Refactored download stream API

* Unskip tests

* Test warnings

* Missing await

* Fixed append tests

* Fixed page tests

* Pylint

* Py2.7 iter support

* Refactor downloaders

* Added missing async module functions

* Updated release notes

* Keyword params

* Documented more keyword options

* Added module functions to exports

* Fixed merge

* Fix for Files download stream

* Pylint fix

* Updated File stream downloads

* Fix tests

* Updated error message

* Added Files release notes

* Pylint fix

* Download stream error handling

* Fixed some tests

* Design Pipeline ownership (#7981)

* Initital Commit

* Pipeline Ownership

* slight modifications

* pipeline ownership for queues and files

* fixes tests, adds docstrings to wrapper classes

* pylint

* Batching APIs Raise on Any Failure (#7963)

* Raise on Single Failure

* some changes

* lint

* some changes

* oops

* comments + lint

* some optimization

* history

* comments

* lint

* recording

* Plug HttpLoggingPolicy to Storage (#8081)

* Plug HttpLoggingPolicy to Storage

* Update dependencies

* Skip depends job for Storage

* Fix shared req

* [storage] make storage files _internal (#7949)

* blobs internal

* files internal

* queues internal

* some fixes to docstrings

* update history.md

* fix async test import

* some file mypy fixes

* some queue mypy fixes

* some blob mypy fixes

* fix mistake in type annot

* history edits

* Merge fix

* Revert "Merge fix"

This reverts commit 748fbd6.

* Better merge fix

* fix some tests (#8100)

* Fix tests (#8103)

* minor fix special char

* fix auth

* comments

* [Storage] Bumped version + Pipeline fix (#8089)

* Bumped version

* Docs tweak

* Re-bumped version

* Added 3.8 classifier

* Added proxy policy to pipeline

* Synced base clients

* Added batch exception

* Comment the echo check to workaround batch headers issue (#8118)

* Fix 8091: expose generated enum to customers (#8117)

* Fix #8091

* Add one in file
fengzhou-msft pushed a commit that referenced this pull request Nov 5, 2019
* Fix SubStream to respect IOBase protocol (#7843)

* removes NoRetry policy (#7845)

* [storage] Makes signed_identifiers a required param (#7844)

* makes signed_identifiers a required param

* fix tests and update history

* removes unecessary check

* fix files and queues tests

* [storage] Changes `file_permission_key` param to `permission_key` (#7841)

* changes file_permission_key param to permission_key

* update history

* Change the param directive to keyword in docstrings (#7855)

* Change the directive to keyword in docstrings

* pylint :(

* [storage] Adds from_string to models (#7870)

* adds from_string to models

* fixes to naming and history

* remove _str in docstrings

* pylint

* [storage-file] await async poller (#7872)

* await files async poller

* fix docstrings

* fix type hints

* [File and Queue] Client constructors (#7853)

* from_queue_url

* file client constructors

* update CHANGELOG

* fix test

* lint fix

* some doc changes

* queue docs

* blob docs

* [storage-queue] rename queue messages (#7895)

* rename queue messages

* update history

* [Storage-Queue][Storage-File] Kwargify positional params (#7877)

* Queue kwargification

* kwargify Files

* small fix

* minor fix

* comments + lint

* some changes

* pylint

* [storage-file, queue] moves param protocol to kwargs in gen_shared_access_signature() (#7897)

* moves protocol in gen_sas to kwargs

* delete whitespace

* [storage] Unexposes models from aio (#7881)

* unexposes models from aio

* unexpose all except clients and pylint fixes

* update history

* unexpose sync paged models

* update history phrasing

* [Storage-queue] Allow None message encode policy (#7898)

* Don't expose NoEncode policies

* Support None value for encode/decode policies

* Updated tests

* Updated sync encryption tests

* Updated async encryption tests

* Updated async encoding tests

* Fixed test

* Fix test

* Updated release notes

* Rename enqueue_message to send_messgae (#7928)

* [storage-blob, queue] Renames Logging to <ServiceName>AnalyticsLogging (#7921)

* renames logging to ServiceAnalyticsLogging

* changes param logging to analytics_logging

* fixes tests due to param name change

* [Blob][File][Encryption]Fix Download Encrypted Blob/File Bug (#7965)

#7957

* Consolidate offset - length behavior in files (#7942)

* Initital Commit

* length changes

* some test changes

* some more changes

* history.md

* Update a couple of recrodings

* comments address

* ops

* Stop modifying internal_response body (#7958)

* Stop modifying internal body

* Replicate for queues and files too

* [Storage] Relocated SAS generation (#7955)

* Expose account name on clients

* Updated blob sas gen

* Updated file sas gen

* Update queue sas gen

* Updated docstrings

* Some pylint fixes

* More pylint

* More pylint

* Reverted auth change

* Added release notes

* Fixed kwarg docstring

* [Storage] Updated close handles (#7940)

* Updated close handles

* Pylint fixes

* Fix tests

* Review feedback

* Missing recursive parameter

* Added release notes

* [Storage] Conditional etag parameters (#8047)

* Updated clients

* Process conditional headers

* Updated tests

* Fixed test

* Updated error scenarios

* Updated release notes

* Revert skipped tests

* [Storage] Download stream refactor (#7848)

* Refactored download stream API

* Unskip tests

* Test warnings

* Missing await

* Fixed append tests

* Fixed page tests

* Pylint

* Py2.7 iter support

* Refactor downloaders

* Added missing async module functions

* Updated release notes

* Keyword params

* Documented more keyword options

* Added module functions to exports

* Fixed merge

* Fix for Files download stream

* Pylint fix

* Updated File stream downloads

* Fix tests

* Updated error message

* Added Files release notes

* Pylint fix

* Download stream error handling

* Fixed some tests

* Design Pipeline ownership (#7981)

* Initital Commit

* Pipeline Ownership

* slight modifications

* pipeline ownership for queues and files

* fixes tests, adds docstrings to wrapper classes

* pylint

* Batching APIs Raise on Any Failure (#7963)

* Raise on Single Failure

* some changes

* lint

* some changes

* oops

* comments + lint

* some optimization

* history

* comments

* lint

* recording

* Plug HttpLoggingPolicy to Storage (#8081)

* Plug HttpLoggingPolicy to Storage

* Update dependencies

* Skip depends job for Storage

* Fix shared req

* [storage] make storage files _internal (#7949)

* blobs internal

* files internal

* queues internal

* some fixes to docstrings

* update history.md

* fix async test import

* some file mypy fixes

* some queue mypy fixes

* some blob mypy fixes

* fix mistake in type annot

* history edits

* Merge fix

* Revert "Merge fix"

This reverts commit 748fbd6.

* Better merge fix

* fix some tests (#8100)

* Fix tests (#8103)

* minor fix special char

* fix auth

* comments

* [Storage] Bumped version + Pipeline fix (#8089)

* Bumped version

* Docs tweak

* Re-bumped version

* Added 3.8 classifier

* Added proxy policy to pipeline

* Synced base clients

* Added batch exception

* Comment the echo check to workaround batch headers issue (#8118)

* Fix 8091: expose generated enum to customers (#8117)

* Fix #8091

* Add one in file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocking-release Blocks release P0 Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants