FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Switch from Sentinel types to Enums by pgjones · Pull Request #154 · python-hyper/h11 · GitHub

Switch from Sentinel types to Enums - #154

Open
pgjones wants to merge 1 commit into
python-hyper:masterfrom
pgjones:master
Open

Switch from Sentinel types to Enums#154
pgjones wants to merge 1 commit into
python-hyper:masterfrom
pgjones:master

Conversation

pgjones commented Aug 25, 2022
edited
Loading

Copy link
Copy Markdown
Member

The latter are much easier to work with when type hinting and can be
used successfully with mypyc, whereas the former are sadly very
difficult in both aspects.

This loses the nice property of type(NEED_DATA) is NEED_DATA (as
expanded on in the deleted docs section). However, I don't think this
is widely used in practice.

Closes #153. See also #8 for reasoning behind the introduction of the type property.

@njsmith what do you think about this?

pgjones force-pushed the master branch 3 times, most recently from c42389e to df6649d Compare August 25, 2022 11:02
The latter are much easier to work with when type hinting and can be
used successfully with mypyc, whereas the former are sadly very
difficult in both aspects.

This loses the nice property of `type(NEED_DATA) is NEED_DATA` (as
expanded on in the deleted docs section). However, I don't think this
is widely used in practice.

Copy link
Copy Markdown
Contributor

This looks to me like a breaking API change that would impact Uvicorn and HTTPX.

I do prefer the API, tho.

Kludex commented Sep 1, 2022

Copy link
Copy Markdown
Contributor

I can check this on Uvicorn tonight. jfyk

Copy link
Copy Markdown
Contributor

@Kludex Sure thing. Permalinks to relevant parts of code in the comment above. (Tho I can see that's not obvious.)

Kludex commented Oct 30, 2022

Copy link
Copy Markdown
Contributor

This was merged on httpcore: encode/httpcore#579

Kludex commented Oct 30, 2022

Copy link
Copy Markdown
Contributor

The only problem on uvicorn was:

            event = h11.InformationalResponse(
                status_code=100, headers=[], reason="Continue"
            )

Issue:

uvicorn/protocols/http/h11_impl.py:537: error: Argument "headers" to "InformationalResponse" has incompatible type "List[<nothing>]"; expected "Union[Headers, List[Tuple[bytes, bytes]], List[Tuple[str, str]]]"  [arg-type]
uvicorn/protocols/http/h11_impl.py:537: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
uvicorn/protocols/http/h11_impl.py:537: note: Consider using "Sequence" instead, which is covariant
Found 1 error in 1 file (checked 53 source files)

I've used this branch on uvicorn to check the changes I'd need: https://github.com/encode/uvicorn/pull/1744/files

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

provide a new version of Connection that returns/accepts enum types for PAUSED/NEED_DATA our_role their_role etc

3 participants


Back | FazBrowse Home | New Git URL