[Storage] Download/Upload Blob APIs Fixes for Transport Compatibility #40490
Conversation
| @@ -45,10 +45,6 @@ async def retry_hook(settings, **kwargs): | |||
| async def is_checksum_retry(response): | |||
| # retry if invalid content md5 | |||
| if response.context.get('validate_content', False) and response.http_response.headers.get('content-md5'): | |||
| try: | |||
There was a problem hiding this comment.
Adapted sync code to async upload is_checksum_retry
sdk/storage/azure-storage-blob/azure/storage/blob/aio/_download_async.py
Outdated
Show resolved
Hide resolved
| await data.response.load_body() | ||
| content = cast(bytes, data.response.body()) | ||
| if hasattr(data.response, "read"): | ||
| content = b"".join([d async for d in data]) |
There was a problem hiding this comment.
__anext__ will automatically call read, so this represents any transport that supports read and should also be compatible with decompression
| if hasattr(data.response, "read"): | ||
| content = b"".join([d async for d in data]) | ||
| else: | ||
| await data.response.load_body() |
There was a problem hiding this comment.
decompression could get tricky here
|
API change check API changes are not detected in this pull request. |
|
|
||
| @pytest.mark.live_test_only | ||
| @BlobPreparer() | ||
| async def test_core_transport(self, **kwargs): |
There was a problem hiding this comment.
Need to fix these tests by mocking the core transport in order to remove the mark live test only.
In test_helpers_async.py, the MockAsyncResponse is required to have a raw attribute that is closeable. The AsyncioRequestsTransport has several attributes populated, and the content read is treated like a file pointer, which needs to be cleaned up.
This is why i've switched to using just the AsyncioRequestsTransport for now. If we do decide to not mock this object directly, there needs to be a call to _setup() and container_name becomes self.container_name.
| @@ -46,8 +46,11 @@ | |||
| async def process_content(data: Any, start_offset: int, end_offset: int, encryption: Dict[str, Any]) -> bytes: | |||
| if data is None: | |||
| raise ValueError("Response cannot be None.") | |||
| await data.response.load_body() | |||
| content = cast(bytes, data.response.body()) | |||
| if hasattr(data.response, "load_body"): | |||
There was a problem hiding this comment.
Is try-except necessary here? File share already has try-except built-in the process_content.
There was a problem hiding this comment.
Looks like transports support either read or load_body, so this should cover all the cases regardless.
weirongw23-msft
requested review from
annatisch,
jalauzon-msft and
vincenttran-msft
as code owners
Need a way to figure out how to mock
AsyncioRequestsTransport, then remove the live test only pytest marker