-
-
Notifications
You must be signed in to change notification settings - Fork 845
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
ASGI: Wait for response to complete before sending disconnect message #919
Changes from all 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 |
---|---|---|
|
@@ -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
trio
might 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
trio
package 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_CHECKING
does actually help here. Iftrio
(actually,trio-typing
) is not installed, mypy is still happy because we use--ignore-missing-imports
so it just ignoresimport trio
andtrio.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
?