-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add timeout in aiohttp.close #41374
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
Add timeout in aiohttp.close #41374
Changes from all commits
dc0c874
b1d0564
99049de
8cd2024
31361f3
ca97c90
a626796
17eb33d
b8a1a25
60e2d08
1027157
bc155e9
4f0cb37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,6 +156,8 @@ | |
| "spinup", | ||
| "cibuildwheel", | ||
| "aoai", | ||
| "aiotest", | ||
| "aiotests", | ||
| "pyprojects", | ||
| "certifi", | ||
| "cffi", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # ------------------------------------------------------------------------- | ||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # Licensed under the MIT License. See LICENSE.txt in the project root for | ||
| # license information. | ||
| # ------------------------------------------------------------------------- | ||
| import sys | ||
| import pytest | ||
| import logging | ||
| from azure.storage.blob.aio import BlobServiceClient | ||
|
|
||
|
|
||
| @pytest.mark.skipif( | ||
| sys.version_info < (3, 11), reason="ssl_shutdown_timeout in aiohttp only takes effect on Python 3.11+" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to skip 3.9/3.10? Ideally, we see no warnings logged for those versions, too.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see errors when using aiohttp 3.12.7 with py 3.9.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What errors? Things should still work with Python 3.9+ since we are only adding a timeout to Also, let's
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aiohttp hangs in Python 3.9 (compared to a 30-second delay in 3.11). In 3.9, we have to forcefully stop aiohttp, and an error from aiohttp appeared during my test on Python 3.9.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't able to reproduce an error locally with 3.9/3.10. Did you see this on CI?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see this on py3.9 w/ timeout in azure-core.aiohttp added: Exception ignored in: <function _ProactorBasePipeTransport.del at 0x000002255D531670>
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I think I'm not seeing it because I'm running on WSL. Looks like that ^ only shows up in windows.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that runtime exception show up in windows/py3.9 even without the timeout changes in this PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. aiohttp 3.12.7 does not solve the problem on py 3.9. (I did not test py 3.10) |
||
| ) | ||
| @pytest.mark.live_test_only | ||
| @pytest.mark.asyncio | ||
| async def test_download_blob_aiohttp(caplog): | ||
| logger = logging.getLogger(__name__) | ||
| AZURE_STORAGE_CONTAINER_NAME = "aiotests" | ||
| account_url = "https://aiotests.blob.core.windows.net" | ||
| blob_service_client = BlobServiceClient(account_url=account_url) | ||
| read_path = "aiotest.txt" | ||
|
|
||
| with caplog.at_level(logging.INFO): | ||
| async with blob_service_client: | ||
| blob_client = blob_service_client.get_blob_client(container=AZURE_STORAGE_CONTAINER_NAME, blob=read_path) | ||
|
|
||
| async with blob_client: | ||
| stream = await blob_client.download_blob() | ||
| data = await stream.readall() | ||
| logger.info(f"Blob size: {len(data)}") | ||
|
|
||
| assert all("Error" not in message for message in caplog.messages) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # ------------------------------------------------------------------------- | ||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # Licensed under the MIT License. See LICENSE.txt in the project root for | ||
| # license information. | ||
| # ------------------------------------------------------------------------- | ||
| import asyncio | ||
| import time | ||
| import pytest | ||
| import types | ||
| from unittest.mock import Mock, patch | ||
|
|
||
| from azure.core.pipeline.transport import ( | ||
| AioHttpTransport, | ||
| ) | ||
|
|
||
|
|
||
| class MockClientSession: | ||
| """Mock aiohttp.ClientSession with a close method that sleeps for 30 seconds.""" | ||
|
|
||
| def __init__(self): | ||
| self._closed = False | ||
|
|
||
| async def __aenter__(self): | ||
| return self | ||
|
|
||
| async def __aexit__(self, *args, **kwargs): | ||
| pass | ||
|
|
||
| async def close(self): | ||
| """Simulate a slow closing session.""" | ||
| self._closed = True | ||
| await asyncio.sleep(30) # Simulate a very slow close operation | ||
|
|
||
| def request(self, *args, **kwargs): | ||
| return asyncio.Future() # Just needs to be awaitable | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_aiohttp_transport_close_timeout(): | ||
| """Test that AioHttpTransport.close() returns within 1 second even if session.close() takes 30 seconds.""" | ||
|
|
||
| # Create transport with our mock session | ||
| mock_session = MockClientSession() | ||
| transport = AioHttpTransport(session=mock_session, session_owner=True) | ||
|
|
||
| # Open the transport to initialize the session | ||
| await transport.open() | ||
|
|
||
| # Time the close operation | ||
| start_time = time.time() | ||
| await transport.close() | ||
| end_time = time.time() | ||
|
|
||
| # Verify close returned in a reasonable time (should be around 0.0001 seconds due to timeout) | ||
| assert end_time - start_time < 0.1, f"Transport close took {end_time - start_time} seconds, should be < 0.1 seconds" | ||
|
|
||
| # Ensure transport's session was set to None | ||
| assert transport.session is None |
Uh oh!
There was an error while loading. Please reload this page.