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

fix AttributeException #11463

Merged
merged 9 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions sdk/core/azure-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

## 1.5.1 (Unreleased)

### Bug fixes

- Fix AttributeException in StreamDownloadGenerator #11462

## 1.5.0 (2020-05-04)

Expand Down
6 changes: 3 additions & 3 deletions sdk/core/azure-core/azure/core/pipeline/transport/_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,12 @@ async def __anext__(self):
else:
await asyncio.sleep(retry_interval)
headers = {'range': 'bytes=' + str(self.downloaded) + '-'}
resp = self.pipeline.run(self.request, stream=True, headers=headers)
if resp.status_code == 416:
resp = await self.pipeline.run(self.request, stream=True, headers=headers)
if resp.http_response.status_code == 416:
raise
chunk = await self.response.internal_response.content.read(self.block_size)
if not chunk:
raise StopIteration()
raise StopAsyncIteration()
self.downloaded += len(chunk)
return chunk
continue
Expand Down
6 changes: 3 additions & 3 deletions sdk/core/azure-core/azure/core/pipeline/transport/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@ class HttpTransport(

@abc.abstractmethod
def send(self, request, **kwargs):
# type: (PipelineRequest, Any) -> PipelineResponse
# type: (HTTPRequestType, Any) -> HTTPResponseType
"""Send the request using this HTTP sender.

:param request: The pipeline request object
:type request: ~azure.core.pipeline.PipelineRequest
:type request: ~azure.core.transport.HTTPRequest
:return: The pipeline response object.
:rtype: ~azure.core.pipeline.PipelineResponse
:rtype: ~azure.core.pipeline.transport.HttpResponse
Copy link
Member

Choose a reason for hiding this comment

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

Are AsyncHttpTransport typing/docstring correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have typing/docstring for AsyncHttpTransport. So it is not incorrect. :)

Copy link
Member

Choose a reason for hiding this comment

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

Might be the opportunity to add them, but yeah optional :p

"""

@abc.abstractmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import abc
from collections.abc import AsyncIterator

from typing import AsyncIterator as AsyncIteratorType, TypeVar, Generic
from typing import AsyncIterator as AsyncIteratorType, TypeVar, Generic, Any
from ._base import (
_HttpResponseBase,
_HttpClientTransportResponse,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def __next__(self):
time.sleep(retry_interval)
headers = {'range': 'bytes=' + str(self.downloaded) + '-'}
resp = self.pipeline.run(self.request, stream=True, headers=headers)
if resp.status_code == 416:
if resp.http_response.status_code == 416:
raise
chunk = next(self.iter_content_func)
if not chunk:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
from azure.core.pipeline.transport import (
HttpRequest,
AsyncHttpResponse,
AsyncHttpTransport,
)
from azure.core.pipeline import AsyncPipeline
from azure.core.pipeline.transport._aiohttp import AioHttpStreamDownloadGenerator
from unittest import mock
import pytest

@pytest.mark.asyncio
async def test_connection_error_response():
class MockTransport(AsyncHttpTransport):
def __init__(self):
self._count = 0

async def __aexit__(self, exc_type, exc_val, exc_tb):
pass
async def close(self):
pass
async def open(self):
pass

async def send(self, request, **kwargs):
request = HttpRequest('GET', 'http://127.0.0.1/')
response = AsyncHttpResponse(request, None)
response.status_code = 200
return response

class MockContent():
def __init__(self):
self._first = True

async def read(self, block_size):
if self._first:
self._first = False
raise ConnectionError
return None

class MockInternalResponse():
def __init__(self):
self.headers = {}
self.content = MockContent()

async def close(self):
pass

class AsyncMock(mock.MagicMock):
async def __call__(self, *args, **kwargs):
return super(AsyncMock, self).__call__(*args, **kwargs)

http_request = HttpRequest('GET', 'http://127.0.0.1/')
pipeline = AsyncPipeline(MockTransport())
http_response = AsyncHttpResponse(http_request, None)
http_response.internal_response = MockInternalResponse()
stream = AioHttpStreamDownloadGenerator(pipeline, http_response)
with mock.patch('asyncio.sleep', new_callable=AsyncMock):
with pytest.raises(StopAsyncIteration):
await stream.__anext__()

@pytest.mark.asyncio
async def test_connection_error_416():
class MockTransport(AsyncHttpTransport):
def __init__(self):
self._count = 0

async def __aexit__(self, exc_type, exc_val, exc_tb):
pass
async def close(self):
pass
async def open(self):
pass

async def send(self, request, **kwargs):
request = HttpRequest('GET', 'http://127.0.0.1/')
response = AsyncHttpResponse(request, None)
response.status_code = 416
return response

class MockContent():
async def read(self, block_size):
raise ConnectionError

class MockInternalResponse():
def __init__(self):
self.headers = {}
self.content = MockContent()

async def close(self):
pass

class AsyncMock(mock.MagicMock):
async def __call__(self, *args, **kwargs):
return super(AsyncMock, self).__call__(*args, **kwargs)

http_request = HttpRequest('GET', 'http://127.0.0.1/')
pipeline = AsyncPipeline(MockTransport())
http_response = AsyncHttpResponse(http_request, None)
http_response.internal_response = MockInternalResponse()
stream = AioHttpStreamDownloadGenerator(pipeline, http_response)
with mock.patch('asyncio.sleep', new_callable=AsyncMock):
with pytest.raises(ConnectionError):
await stream.__anext__()
101 changes: 101 additions & 0 deletions sdk/core/azure-core/tests/test_stream_generator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
import requests
from azure.core.pipeline.transport import (
HttpRequest,
HttpResponse,
HttpTransport,
)
from azure.core.pipeline import Pipeline, PipelineResponse
from azure.core.pipeline.transport._requests_basic import StreamDownloadGenerator
try:
from unittest import mock
except ImportError:
import mock
import pytest

def test_connection_error_response():
class MockTransport(HttpTransport):
def __init__(self):
self._count = 0

def __exit__(self, exc_type, exc_val, exc_tb):
pass
def close(self):
pass
def open(self):
pass

def send(self, request, **kwargs):
request = HttpRequest('GET', 'http://127.0.0.1/')
response = HttpResponse(request, None)
response.status_code = 200
return response

def next(self):
self.__next__()

def __next__(self):
if self._count == 0:
self._count += 1
raise requests.exceptions.ConnectionError

class MockInternalResponse():
def iter_content(self, block_size):
return MockTransport()

def close(self):
pass

http_request = HttpRequest('GET', 'http://127.0.0.1/')
pipeline = Pipeline(MockTransport())
http_response = HttpResponse(http_request, None)
http_response.internal_response = MockInternalResponse()
stream = StreamDownloadGenerator(pipeline, http_response)
with mock.patch('time.sleep', return_value=None):
with pytest.raises(StopIteration):
stream.__next__()

def test_connection_error_416():
class MockTransport(HttpTransport):
def __init__(self):
self._count = 0

def __exit__(self, exc_type, exc_val, exc_tb):
pass
def close(self):
pass
def open(self):
pass

def send(self, request, **kwargs):
request = HttpRequest('GET', 'http://127.0.0.1/')
response = HttpResponse(request, None)
response.status_code = 416
return response

def next(self):
self.__next__()

def __next__(self):
if self._count == 0:
self._count += 1
raise requests.exceptions.ConnectionError

class MockInternalResponse():
def iter_content(self, block_size):
return MockTransport()

def close(self):
pass

http_request = HttpRequest('GET', 'http://127.0.0.1/')
pipeline = Pipeline(MockTransport())
http_response = HttpResponse(http_request, None)
http_response.internal_response = MockInternalResponse()
stream = StreamDownloadGenerator(pipeline, http_response)
with mock.patch('time.sleep', return_value=None):
with pytest.raises(requests.exceptions.ConnectionError):
stream.__next__()