-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Factorize hypercon.Config() creation #3234
base: main
Are you sure you want to change the base?
Conversation
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.
LGTM, thanks Quentin!
@pquentin Looks like potential failures on the certificate tests, might be related? |
Definitely related, thanks. |
@@ -62,7 +76,7 @@ def run_hypercorn_in_thread( | |||
if not ready_event.is_set(): | |||
raise Exception("most likely failed to start server") | |||
|
|||
yield | |||
yield typing.cast(int, parse_url(config.bind[0]).port) |
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.
yield typing.cast(int, parse_url(config.bind[0]).port) | |
port = parse_url(config.bind[0]).port | |
assert port is not None | |
yield port |
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.
When can this happen?
config: hypercorn.Config, app: hypercorn.typing.ASGIFramework | ||
) -> Generator[None, None, None]: | ||
host: str, certs: dict[str, typing.Any] | None, app: hypercorn.typing.ASGIFramework | ||
) -> typing.Iterator[int]: |
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.
Iterator is technically the wrong type, because contextmanager incorrectly accepts an object without .throw
. This bug might get fixed in typeshed
) -> typing.Iterator[int]: | |
) -> Generator[int, None, None] |
I have the same changes ready for #3230, I'll update whichever pull request gets merged last.