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.

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

Update types for h11 v0.14 #579

Merged
merged 20 commits into from Sep 27, 2022
Merged

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Sep 25, 2022

Extends and supersedes #526.

Updates the types in _send_event and recieve_event for compatibility with h11 0.14.0.

Tests and mypy were run locally on h11 0.13.0 and 0.14.0.

Closes #503
Closes #498
Unblocks #509

Differences from #526

Removes cast from the "hot loop"

h11 0.14.0 includes the upstream fix python-hyper/h11@04cc0f7 allowing us to remove the cast and isinstance check from the hot loop which @graingert expressed concerns about in #526 (comment). A cast is still required for the return value as mypy is not type-narrowing on the is clause. We'd probably have to do a weird isinstance/issubclass combination to avoid the cast, which would bring us back to the original concerns about affected performance.

Interestingly, mypy passes with h11 0.13.0 so we may have been able to use this implementation without the upstream change.

Addresses narrower types

Updates the _send_event types to be narrower as mentioned in #526 (comment). If overloads are added to the h11 library, this will allow us to remove an unnecessary null check as noted in #526 (comment).

Bumps minimum h11 version

Bumps the minimum h11 version to 0.13.0 which is required for the h11.Event type. I imagine we could create a conditional stub for the type if support for older versions of h11 is important.

@zanieb
Copy link
Contributor Author

zanieb commented Sep 25, 2022

Let me know if you'd like me to rebase to clean this history up. For now, I've just extended the commits from the original branch for clarity.

@tomchristie
Copy link
Member

Let me know if you'd like me to rebase to clean this history up.

Not needed, no. We have "squash and merge" enforced on this project, so pull requests always end up as our atomic unit of history.

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

A linting fix here...

httpcore/_async/http11.py Outdated Show resolved Hide resolved
httpcore/_sync/http11.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants