-
-
Couldn't load subscription status.
- Fork 968
ASGI: Wait for response to complete before sending disconnect message #919
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
Changes from 4 commits
1cec8df
620bc0c
bf6f194
45f2466
6365262
1979edb
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 |
|---|---|---|
| @@ -1,9 +1,28 @@ | ||
| import typing | ||
| from typing import Callable, Dict, List, Optional, Tuple | ||
|
|
||
| import httpcore | ||
| import sniffio | ||
|
|
||
| from .._content_streams import ByteStream | ||
|
|
||
| if typing.TYPE_CHECKING: # pragma: no cover | ||
| import asyncio | ||
| import trio | ||
|
|
||
| Event = typing.Union[asyncio.Event, trio.Event] | ||
|
|
||
|
|
||
| def create_event() -> "Event": | ||
| if sniffio.current_async_library() == "trio": | ||
| import trio | ||
|
|
||
| return trio.Event() | ||
| else: | ||
| import asyncio | ||
|
|
||
| return asyncio.Event() | ||
|
|
||
|
|
||
| class ASGIDispatch(httpcore.AsyncHTTPTransport): | ||
| """ | ||
|
|
@@ -76,23 +95,27 @@ async def request( | |
| status_code = None | ||
| response_headers = None | ||
| body_parts = [] | ||
| request_complete = False | ||
| response_started = False | ||
| response_complete = False | ||
| response_complete = create_event() | ||
|
|
||
| headers = [] if headers is None else headers | ||
| stream = ByteStream(b"") if stream is None else stream | ||
|
|
||
| request_body_chunks = stream.__aiter__() | ||
|
|
||
| async def receive() -> dict: | ||
| nonlocal response_complete | ||
| nonlocal request_complete, response_complete | ||
|
|
||
| if response_complete: | ||
| if request_complete: | ||
| # Simulate blocking until the response is complete and then disconnect | ||
|
||
| await response_complete.wait() | ||
| return {"type": "http.disconnect"} | ||
|
|
||
| try: | ||
| body = await request_body_chunks.__anext__() | ||
| except StopAsyncIteration: | ||
| request_complete = True | ||
| return {"type": "http.request", "body": b"", "more_body": False} | ||
| return {"type": "http.request", "body": body, "more_body": True} | ||
|
|
||
|
|
@@ -108,23 +131,23 @@ async def send(message: dict) -> None: | |
| response_started = True | ||
|
|
||
| elif message["type"] == "http.response.body": | ||
| assert not response_complete | ||
| assert not response_complete.is_set() | ||
| body = message.get("body", b"") | ||
| more_body = message.get("more_body", False) | ||
|
|
||
| if body and method != b"HEAD": | ||
| body_parts.append(body) | ||
|
|
||
| if not more_body: | ||
| response_complete = True | ||
| response_complete.set() | ||
|
|
||
| try: | ||
| await self.app(scope, receive, send) | ||
| except Exception: | ||
| if self.raise_app_exceptions or not response_complete: | ||
| raise | ||
|
|
||
| assert response_complete | ||
| assert response_complete.is_set() | ||
| assert status_code is not None | ||
| assert response_headers is not None | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ pytest-asyncio | |
| pytest-trio | ||
| pytest-cov | ||
| trio | ||
| trio-typing | ||
|
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 add this dependency? 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. For any trio types to be type-checked this must be installed (the only type being checked is |
||
| trustme | ||
| uvicorn | ||
| seed-isort-config | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so
triomight not be installed.Perhaps we should be doing something like this?...
Also it's not obvious to me that we need to bother with the
if typing.TYPE_CHECKING:dance? We usually only use that if it's required to avoid otherwise circular imports.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried what you're suggesting but unfortunately run into an error like:
Seems like type aliases need to be statically resolvable: https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. We can't rely on the
triopackage necessarily being installed tho.(Even if we're limiting this to
if typing.TYPE_CHECKING:)One other option might be to just under-specify the type in this case, as a lazy
typing.Any. We're not exposing the type info or using that function outside of this scope, so perhaps that's a sensibly practical option?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
if typing.TYPE_CHECKINGdoes actually help here. Iftrio(actually,trio-typing) is not installed, mypy is still happy because we use--ignore-missing-importsso it just ignoresimport trioandtrio.Event().There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomchristie still want me to make that a
typing.Any?