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.

Sign up for GitHub

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

Handle asynchronous cancellations for HTTPConnection instances #802

Closed
wants to merge 11 commits into from
Closed

Handle asynchronous cancellations for HTTPConnection instances #802

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5f55645
Add is_new property for HTTPConnections
karpetrosyan Sep 14, 2023
093f4b8
Optimize
karpetrosyan Sep 14, 2023
49ad393
Typo
karpetrosyan Sep 14, 2023
fbb6904
Update httpcore/_sync/connection.py
karpetrosyan Sep 15, 2023
c46ec09
Update httpcore/_async/connection.py
karpetrosyan Sep 15, 2023
958e402
Fix tests
karpetrosyan Sep 15, 2023
ef814ee
coverage
karpetrosyan Sep 15, 2023
281402d
Merge branch 'master' into handle-cancelled-connections
karpetrosyan Sep 16, 2023
90c39a4
Merge branch 'master' into handle-cancelled-connections
karpetrosyan Sep 23, 2023
ee17897
Merge branch 'master' into handle-cancelled-connections
karpetrosyan Oct 18, 2023
87e526d
changelog
karpetrosyan Oct 19, 2023
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
10 changes: 8 additions & 2 deletions httpcore/_async/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(
self._connect_failed: bool = False
self._request_lock = AsyncLock()
self._socket_options = socket_options
self._is_new = True

async def handle_async_request(self, request: Request) -> Response:
if not self.can_handle_request(request.url.origin):
Expand All @@ -73,6 +74,7 @@ async def handle_async_request(self, request: Request) -> Response:
async with self._request_lock:
if self._connection is None:
try:
self._is_new = False
stream = await self._connect(request)

ssl_object = stream.get_extra_info("ssl_object")
Expand All @@ -94,7 +96,7 @@ async def handle_async_request(self, request: Request) -> Response:
stream=stream,
keepalive_expiry=self._keepalive_expiry,
)
except Exception as exc:
except BaseException as exc:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to be sure that it works when we cancel the asynchronous task.

self._connect_failed = True
raise exc
elif not self._connection.is_available():
Expand Down Expand Up @@ -174,6 +176,8 @@ async def aclose(self) -> None:

def is_available(self) -> bool:
if self._connection is None:
if self._is_new:
return True
# If HTTP/2 support is enabled, and the resulting connection could
# end up as HTTP/2 then we should indicate the connection as being
# available to service multiple requests.
Expand Down Expand Up @@ -201,7 +205,9 @@ def is_closed(self) -> bool:

def info(self) -> str:
if self._connection is None:
return "CONNECTION FAILED" if self._connect_failed else "CONNECTING"
if self._is_new:
return "NEW CONNECTION"
return "CONNECTINION FAILED" if self._connect_failed else "CONNECTING"
karpetrosyan marked this conversation as resolved.
Show resolved Hide resolved
return self._connection.info()

def __repr__(self) -> str:
Expand Down
10 changes: 8 additions & 2 deletions httpcore/_sync/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(
self._connect_failed: bool = False
self._request_lock = Lock()
self._socket_options = socket_options
self._is_new = True

def handle_request(self, request: Request) -> Response:
if not self.can_handle_request(request.url.origin):
Expand All @@ -73,6 +74,7 @@ def handle_request(self, request: Request) -> Response:
with self._request_lock:
if self._connection is None:
try:
self._is_new = False
stream = self._connect(request)

ssl_object = stream.get_extra_info("ssl_object")
Expand All @@ -94,7 +96,7 @@ def handle_request(self, request: Request) -> Response:
stream=stream,
keepalive_expiry=self._keepalive_expiry,
)
except Exception as exc:
except BaseException as exc:
self._connect_failed = True
raise exc
elif not self._connection.is_available():
Expand Down Expand Up @@ -174,6 +176,8 @@ def close(self) -> None:

def is_available(self) -> bool:
if self._connection is None:
if self._is_new:
return True
# If HTTP/2 support is enabled, and the resulting connection could
# end up as HTTP/2 then we should indicate the connection as being
# available to service multiple requests.
Expand Down Expand Up @@ -201,7 +205,9 @@ def is_closed(self) -> bool:

def info(self) -> str:
if self._connection is None:
return "CONNECTION FAILED" if self._connect_failed else "CONNECTING"
if self._is_new:
return "NEW CONNECTION"
return "CONNECTINION FAILED" if self._connect_failed else "CONNECTING"
karpetrosyan marked this conversation as resolved.
Show resolved Hide resolved
return self._connection.info()

def __repr__(self) -> str:
Expand Down
Loading