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

Download sparse blob #7555

Merged

Conversation

xiafu-msft
Copy link
Contributor

@xiafu-msft xiafu-msft commented Oct 2, 2019

Resolves #7295
GitHub issue link Azure/azure-cli#5872

@adxsdk6
Copy link

adxsdk6 commented Oct 2, 2019

Can one of the admins verify this patch?

@xiafu-msft xiafu-msft marked this pull request as ready for review October 2, 2019 20:11
@xiafu-msft xiafu-msft mentioned this pull request Oct 2, 2019
@@ -13,7 +13,6 @@
from .blob_service_client import BlobServiceClient
from .lease import LeaseClient
from ._shared.policies import ExponentialRetry, LinearRetry, NoRetry
from ._shared.downloads import StorageStreamDownloader
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we want to expose it to user I will add it back in the next commit

Copy link
Member

Choose a reason for hiding this comment

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

I think this probably should be exposed - it is the return type of the download operations and if users what to do Type-checking they will need to be able to import this type.
I also think it it's exposed in the Files SDK, we whichever way we go should be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't exposed in File I think, but I assumed we want to expose it

@@ -157,14 +179,16 @@ class StorageStreamDownloader(object): # pylint: disable=too-many-instance-attr
"""

def __init__(
self, service=None,
self, client=None,
Copy link
Contributor Author

@xiafu-msft xiafu-msft Oct 2, 2019

Choose a reason for hiding this comment

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

if you are fine with seperating download.py for file and blob, we can remove client from constructor and only keep clients. (Currenly client is used to perform download in blob_operations, clients is used to get page_blob client, and perform get_page_ranges operation.)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we should be some refactoring here - having both client and clients seems odd.
Though if it's not public facing it can be refactored at a later date.

Copy link
Member

Choose a reason for hiding this comment

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

Rather than have a public attribute called clients - could we possibly rename this to something like generated_api and make it private?
That way if we refactor or rename it later, it's already private and we can do what we like with it. I also feel the name clients is confusing.

def __init__(client=None, generated_api=None, ..., **kwargs):
    self.client = client
    self._generated_api = generated_api

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about just keep clients and remove client?

@rakshith91 rakshith91 marked this pull request as ready for review October 2, 2019 23:19
@zezha-msft
Copy link
Contributor

            self.assertEqual(byte, 0)

add comment to explain except


Refers to: sdk/storage/azure-storage-blob/tests/test_page_blob.py:1579 in 4327bf1. [](commit_id = 4327bf1, deletion_comment = False)

Copy link
Contributor

@zezha-msft zezha-msft left a comment

Choose a reason for hiding this comment

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

🕐

Copy link
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.

Thanks @xiafu-msft - the changes look good!
Just a couple of tiny things :)

@@ -157,14 +179,16 @@ class StorageStreamDownloader(object): # pylint: disable=too-many-instance-attr
"""

def __init__(
self, service=None,
self, client=None,
Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we should be some refactoring here - having both client and clients seems odd.
Though if it's not public facing it can be refactored at a later date.

@@ -157,14 +179,16 @@ class StorageStreamDownloader(object): # pylint: disable=too-many-instance-attr
"""

def __init__(
self, service=None,
self, client=None,
Copy link
Member

Choose a reason for hiding this comment

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

Rather than have a public attribute called clients - could we possibly rename this to something like generated_api and make it private?
That way if we refactor or rename it later, it's already private and we can do what we like with it. I also feel the name clients is confusing.

def __init__(client=None, generated_api=None, ..., **kwargs):
    self.client = client
    self._generated_api = generated_api

@@ -13,7 +13,6 @@
from .blob_service_client import BlobServiceClient
from .lease import LeaseClient
from ._shared.policies import ExponentialRetry, LinearRetry, NoRetry
from ._shared.downloads import StorageStreamDownloader
Copy link
Member

Choose a reason for hiding this comment

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

I think this probably should be exposed - it is the return type of the download operations and if users what to do Type-checking they will need to be able to import this type.
I also think it it's exposed in the Files SDK, we whichever way we go should be consistent.

lmazuel added a commit that referenced this pull request Oct 3, 2019
@lmazuel lmazuel added the P0 label Oct 3, 2019
lmazuel added a commit that referenced this pull request Oct 3, 2019
Copy link
Contributor

@zezha-msft zezha-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

@xiafu-msft xiafu-msft merged commit e9ef9cf into Azure:feature/storage-preview4 Oct 3, 2019
zezha-msft pushed a commit that referenced this pull request Oct 8, 2019
* Update msrest and regenerate swagger. (#7308)

* Update msrest and regenerate swagger.

* override to get green CI.

* Fix to have no content type when request body is empty (#7343)

* Correct return types for list_secrets, _versions (#7268)

* More useful exceptions for Key Vault errors (#7086)

* Fix to have no content type when request body is empty

* [Blob] Set tier and rehydrate (#7269)

* [BlockBlob][Tier]Add BlobTier Support For CopyBlob/PutBlob/PutBlockList

* [BlockBlob][Tier]Add Recordings for Standard BlobTier Support

* [BlockBlob][Rehydrate]Enable Rehydrate Priority for Copy/SetTier

* [BlockBlob][Rehydrate]Recordings for Rehydrate Priority for Copy/SetTier

* [Blob] Echo client (#7350)

* [EchoClientId]Verify Client Request ID in Response Same as in Request

* [BlockBlob][Rehydrate]Fix Recordings

* Refactor max_connections to max_concurrency (#7531)

* Rename max_connections to max_concurrency

* Other non breaking changes

* comment changes

* Use generated version as constant API version (#7547)

* Re-recording from #7555 (#7585)

* Storage batch API (#7387)

* delete_blobs POC

* Storage POC WIP

* Storage POC async

* Storage WIP

* Storage clean-up

* Storage options

* Autorest remark

* Use config option

* SetBlobTier WIP

* Working and tested delete_blobs + options

* Set standard tier with tests

* Adapt tests for playback mode

* Move batch_send to common mixin

* Python 2.7 compat

* Fix snapshot bug

* Re-record

* Pass kwargs to batch

* Clean x-ms-client-request-id from body if multipart response

* remove duplicated comp=tier

* Disable batch tests on Python 2.7

* pylint

* Batch ChangeLog (#7593)

* [Storage Blob] Regenerate Swagger (#7572)

* Regenerate Swagger

* small fix

* adjusts literalincludes for msft docs (#7599)

* Download sparse blob (#7555)

* [PageBlob]Optimize Download Sparse File

* Breaking Changes - Permission models (#7517)

* Rename Permission related models to SasPermissions

* Rewrite Permission Models

* refactor tests

* some tests

* comment changes

* remove length checks

* QueuePermissions

* File Permissions

* blobs minor tweaks

* minor fix

* pylint fix

* pylint

* Fix batch docstrings (#7600)

* Fix batch docstrings

* pylint

* [Rename]rename max_connections to max_concurrency (#7606)

* small edits to lease client docs (#7550)

* small edits to lease client docs

* whitespace

* Setup identity for unified pipelines. (#7578)

* Setup keyvault for unified pipelines. (#7579)

* Setup eventhubs for publishing via unified pipelines. (#7580)

* Setup eventhubs for publishing via unified pipelines.

* Fixed missing punctuation.

* Setup cosmos for release via unified pipelines. (#7581)

* bump CI

* Manual network september (#7576)

* generated files

* adding 2019-07-01

* history and version

* rerun tests

* updated version

* removed subscription id from tests

* rerun tests again

* replaced subscription id

* [AutoPR] reservations/resource-manager (#7569)

* [AutoPR reservations/resource-manager] [Hub Generated] Review request for Microsoft.Capacity to add version preview/2019-04-01 (#7251)

* Generated from f19a2e5b7f384018b74b21b7b8b8782d95b456f9

fixed x-ms-enum value

* Generated from 2f56008117d578bec6cc8b8c832926f45a0fe52e

fixed catalog definition

* Generated from cac978330e8c7b9583812a735cfeac97fb267056

reverted breaking operation id change

* added version and history

* [AutoPR] sql/resource-manager (#7535)

* Generated from 9402dbf3fb3d5fffcb80f501b1fc857ff97ab723 (#7329)

Fixing PR validation errors

* regenerated

* history and version

* Setup app configuration with unified pipelines. (#7582)

* [AutoPR] resources/resource-manager (#7549)

* regenerated

* updated history and version

* updated date

* Correct import_key parameter type (#7590)

* add doc for tracing (#7616)

* Add a from_blob_url method (#7567)

* Add a from_blob_url method

* Update tests

* Recordings - common blob

* Revert "Recordings - common blob"

This reverts commit ad07f34c25f7662cbaed8b33ac05a1a6f6270dae.

* comment changes

* undo get_client changes

* redo recordings

* from_container_url

* fix tests

* tests fix for container url

* pylint fix

* Fix some docstring

* docstrings

* Doc imprvment for Storage (#7601)

* Docstring improvement

* File docstring

* Queue docstrings

* More doc fixes

* More doc fixes

* Revert "small edits to lease client docs (#7550)" (#7631)

This reverts commit ed20b58.

* kwarg-ify methods (#7611)

* changeset1

* changeset-2

* changeset-3

* changeset-4

* minor fix

* some final tweaks

* pylint

* minor fixes

* max_concurrency

* lease_id

* pylint

* fix

* [Storage] Consolidate offset and range parameters (#7598)

* Updated blob sync tests

* Offset refactor APIS (#19)

* rename to offset and length

* fix

* more changes

* Updated blob sync tests

* Temp block async tests

* Fix upload page behaviour

* Fix clear page behaviour

* Update from_url offset behaviour

* Update page ranges

* Fix download blob behaviour

* Some cleanup

* Fixed page size

* More test fixes

* Some more fixes

* Fixed page tests

* Fixed encryption tests

* Fix common test

* append anf page blob async

* some more test fixes

* Fix live tests

* more changes async

* Fix sparse blob test

* Last tests

* update docstrings

* pylint

* Some Final tweaks (#7653)

* Some final tweaks

* comments

* Fix live tests (#7665)

* fix live tests

* oops

* few changes
YijunXieMS pushed a commit to YijunXieMS/azure-sdk-for-python that referenced this pull request Oct 9, 2019
* Update msrest and regenerate swagger. (Azure#7308)

* Update msrest and regenerate swagger.

* override to get green CI.

* Fix to have no content type when request body is empty (Azure#7343)

* Correct return types for list_secrets, _versions (Azure#7268)

* More useful exceptions for Key Vault errors (Azure#7086)

* Fix to have no content type when request body is empty

* [Blob] Set tier and rehydrate (Azure#7269)

* [BlockBlob][Tier]Add BlobTier Support For CopyBlob/PutBlob/PutBlockList

* [BlockBlob][Tier]Add Recordings for Standard BlobTier Support

* [BlockBlob][Rehydrate]Enable Rehydrate Priority for Copy/SetTier

* [BlockBlob][Rehydrate]Recordings for Rehydrate Priority for Copy/SetTier

* [Blob] Echo client (Azure#7350)

* [EchoClientId]Verify Client Request ID in Response Same as in Request

* [BlockBlob][Rehydrate]Fix Recordings

* Refactor max_connections to max_concurrency (Azure#7531)

* Rename max_connections to max_concurrency

* Other non breaking changes

* comment changes

* Use generated version as constant API version (Azure#7547)

* Re-recording from Azure#7555 (Azure#7585)

* Storage batch API (Azure#7387)

* delete_blobs POC

* Storage POC WIP

* Storage POC async

* Storage WIP

* Storage clean-up

* Storage options

* Autorest remark

* Use config option

* SetBlobTier WIP

* Working and tested delete_blobs + options

* Set standard tier with tests

* Adapt tests for playback mode

* Move batch_send to common mixin

* Python 2.7 compat

* Fix snapshot bug

* Re-record

* Pass kwargs to batch

* Clean x-ms-client-request-id from body if multipart response

* remove duplicated comp=tier

* Disable batch tests on Python 2.7

* pylint

* Batch ChangeLog (Azure#7593)

* [Storage Blob] Regenerate Swagger (Azure#7572)

* Regenerate Swagger

* small fix

* adjusts literalincludes for msft docs (Azure#7599)

* Download sparse blob (Azure#7555)

* [PageBlob]Optimize Download Sparse File

* Breaking Changes - Permission models (Azure#7517)

* Rename Permission related models to SasPermissions

* Rewrite Permission Models

* refactor tests

* some tests

* comment changes

* remove length checks

* QueuePermissions

* File Permissions

* blobs minor tweaks

* minor fix

* pylint fix

* pylint

* Fix batch docstrings (Azure#7600)

* Fix batch docstrings

* pylint

* [Rename]rename max_connections to max_concurrency (Azure#7606)

* small edits to lease client docs (Azure#7550)

* small edits to lease client docs

* whitespace

* Setup identity for unified pipelines. (Azure#7578)

* Setup keyvault for unified pipelines. (Azure#7579)

* Setup eventhubs for publishing via unified pipelines. (Azure#7580)

* Setup eventhubs for publishing via unified pipelines.

* Fixed missing punctuation.

* Setup cosmos for release via unified pipelines. (Azure#7581)

* bump CI

* Manual network september (Azure#7576)

* generated files

* adding 2019-07-01

* history and version

* rerun tests

* updated version

* removed subscription id from tests

* rerun tests again

* replaced subscription id

* [AutoPR] reservations/resource-manager (Azure#7569)

* [AutoPR reservations/resource-manager] [Hub Generated] Review request for Microsoft.Capacity to add version preview/2019-04-01 (Azure#7251)

* Generated from f19a2e5b7f384018b74b21b7b8b8782d95b456f9

fixed x-ms-enum value

* Generated from 2f56008117d578bec6cc8b8c832926f45a0fe52e

fixed catalog definition

* Generated from cac978330e8c7b9583812a735cfeac97fb267056

reverted breaking operation id change

* added version and history

* [AutoPR] sql/resource-manager (Azure#7535)

* Generated from 9402dbf3fb3d5fffcb80f501b1fc857ff97ab723 (Azure#7329)

Fixing PR validation errors

* regenerated

* history and version

* Setup app configuration with unified pipelines. (Azure#7582)

* [AutoPR] resources/resource-manager (Azure#7549)

* regenerated

* updated history and version

* updated date

* Correct import_key parameter type (Azure#7590)

* add doc for tracing (Azure#7616)

* Add a from_blob_url method (Azure#7567)

* Add a from_blob_url method

* Update tests

* Recordings - common blob

* Revert "Recordings - common blob"

This reverts commit ad07f34c25f7662cbaed8b33ac05a1a6f6270dae.

* comment changes

* undo get_client changes

* redo recordings

* from_container_url

* fix tests

* tests fix for container url

* pylint fix

* Fix some docstring

* docstrings

* Doc imprvment for Storage (Azure#7601)

* Docstring improvement

* File docstring

* Queue docstrings

* More doc fixes

* More doc fixes

* Revert "small edits to lease client docs (Azure#7550)" (Azure#7631)

This reverts commit ed20b58.

* kwarg-ify methods (Azure#7611)

* changeset1

* changeset-2

* changeset-3

* changeset-4

* minor fix

* some final tweaks

* pylint

* minor fixes

* max_concurrency

* lease_id

* pylint

* fix

* [Storage] Consolidate offset and range parameters (Azure#7598)

* Updated blob sync tests

* Offset refactor APIS (Azure#19)

* rename to offset and length

* fix

* more changes

* Updated blob sync tests

* Temp block async tests

* Fix upload page behaviour

* Fix clear page behaviour

* Update from_url offset behaviour

* Update page ranges

* Fix download blob behaviour

* Some cleanup

* Fixed page size

* More test fixes

* Some more fixes

* Fixed page tests

* Fixed encryption tests

* Fix common test

* append anf page blob async

* some more test fixes

* Fix live tests

* more changes async

* Fix sparse blob test

* Last tests

* update docstrings

* pylint

* Some Final tweaks (Azure#7653)

* Some final tweaks

* comments

* Fix live tests (Azure#7665)

* fix live tests

* oops

* few changes
@xiafu-msft xiafu-msft deleted the download-sparse-blob branch November 1, 2019 00:59
fengzhou-msft pushed a commit that referenced this pull request Nov 5, 2019
* Update msrest and regenerate swagger. (#7308)

* Update msrest and regenerate swagger.

* override to get green CI.

* Fix to have no content type when request body is empty (#7343)

* Correct return types for list_secrets, _versions (#7268)

* More useful exceptions for Key Vault errors (#7086)

* Fix to have no content type when request body is empty

* [Blob] Set tier and rehydrate (#7269)

* [BlockBlob][Tier]Add BlobTier Support For CopyBlob/PutBlob/PutBlockList

* [BlockBlob][Tier]Add Recordings for Standard BlobTier Support

* [BlockBlob][Rehydrate]Enable Rehydrate Priority for Copy/SetTier

* [BlockBlob][Rehydrate]Recordings for Rehydrate Priority for Copy/SetTier

* [Blob] Echo client (#7350)

* [EchoClientId]Verify Client Request ID in Response Same as in Request

* [BlockBlob][Rehydrate]Fix Recordings

* Refactor max_connections to max_concurrency (#7531)

* Rename max_connections to max_concurrency

* Other non breaking changes

* comment changes

* Use generated version as constant API version (#7547)

* Re-recording from #7555 (#7585)

* Storage batch API (#7387)

* delete_blobs POC

* Storage POC WIP

* Storage POC async

* Storage WIP

* Storage clean-up

* Storage options

* Autorest remark

* Use config option

* SetBlobTier WIP

* Working and tested delete_blobs + options

* Set standard tier with tests

* Adapt tests for playback mode

* Move batch_send to common mixin

* Python 2.7 compat

* Fix snapshot bug

* Re-record

* Pass kwargs to batch

* Clean x-ms-client-request-id from body if multipart response

* remove duplicated comp=tier

* Disable batch tests on Python 2.7

* pylint

* Batch ChangeLog (#7593)

* [Storage Blob] Regenerate Swagger (#7572)

* Regenerate Swagger

* small fix

* adjusts literalincludes for msft docs (#7599)

* Download sparse blob (#7555)

* [PageBlob]Optimize Download Sparse File

* Breaking Changes - Permission models (#7517)

* Rename Permission related models to SasPermissions

* Rewrite Permission Models

* refactor tests

* some tests

* comment changes

* remove length checks

* QueuePermissions

* File Permissions

* blobs minor tweaks

* minor fix

* pylint fix

* pylint

* Fix batch docstrings (#7600)

* Fix batch docstrings

* pylint

* [Rename]rename max_connections to max_concurrency (#7606)

* small edits to lease client docs (#7550)

* small edits to lease client docs

* whitespace

* Setup identity for unified pipelines. (#7578)

* Setup keyvault for unified pipelines. (#7579)

* Setup eventhubs for publishing via unified pipelines. (#7580)

* Setup eventhubs for publishing via unified pipelines.

* Fixed missing punctuation.

* Setup cosmos for release via unified pipelines. (#7581)

* bump CI

* Manual network september (#7576)

* generated files

* adding 2019-07-01

* history and version

* rerun tests

* updated version

* removed subscription id from tests

* rerun tests again

* replaced subscription id

* [AutoPR] reservations/resource-manager (#7569)

* [AutoPR reservations/resource-manager] [Hub Generated] Review request for Microsoft.Capacity to add version preview/2019-04-01 (#7251)

* Generated from f19a2e5b7f384018b74b21b7b8b8782d95b456f9

fixed x-ms-enum value

* Generated from 2f56008117d578bec6cc8b8c832926f45a0fe52e

fixed catalog definition

* Generated from cac978330e8c7b9583812a735cfeac97fb267056

reverted breaking operation id change

* added version and history

* [AutoPR] sql/resource-manager (#7535)

* Generated from 9402dbf3fb3d5fffcb80f501b1fc857ff97ab723 (#7329)

Fixing PR validation errors

* regenerated

* history and version

* Setup app configuration with unified pipelines. (#7582)

* [AutoPR] resources/resource-manager (#7549)

* regenerated

* updated history and version

* updated date

* Correct import_key parameter type (#7590)

* add doc for tracing (#7616)

* Add a from_blob_url method (#7567)

* Add a from_blob_url method

* Update tests

* Recordings - common blob

* Revert "Recordings - common blob"

This reverts commit ad07f34c25f7662cbaed8b33ac05a1a6f6270dae.

* comment changes

* undo get_client changes

* redo recordings

* from_container_url

* fix tests

* tests fix for container url

* pylint fix

* Fix some docstring

* docstrings

* Doc imprvment for Storage (#7601)

* Docstring improvement

* File docstring

* Queue docstrings

* More doc fixes

* More doc fixes

* Revert "small edits to lease client docs (#7550)" (#7631)

This reverts commit ed20b58.

* kwarg-ify methods (#7611)

* changeset1

* changeset-2

* changeset-3

* changeset-4

* minor fix

* some final tweaks

* pylint

* minor fixes

* max_concurrency

* lease_id

* pylint

* fix

* [Storage] Consolidate offset and range parameters (#7598)

* Updated blob sync tests

* Offset refactor APIS (#19)

* rename to offset and length

* fix

* more changes

* Updated blob sync tests

* Temp block async tests

* Fix upload page behaviour

* Fix clear page behaviour

* Update from_url offset behaviour

* Update page ranges

* Fix download blob behaviour

* Some cleanup

* Fixed page size

* More test fixes

* Some more fixes

* Fixed page tests

* Fixed encryption tests

* Fix common test

* append anf page blob async

* some more test fixes

* Fix live tests

* more changes async

* Fix sparse blob test

* Last tests

* update docstrings

* pylint

* Some Final tweaks (#7653)

* Some final tweaks

* comments

* Fix live tests (#7665)

* fix live tests

* oops

* few changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants