h11 icon indicating copy to clipboard operation
h11 copied to clipboard

Switch from Sentinel types to Enums

Open pgjones opened this issue 3 years ago • 5 comments

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 avatar Aug 25 '22 10:08 pgjones

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

I do prefer the API, tho.

lovelydinosaur avatar Sep 01 '22 10:09 lovelydinosaur

I can check this on Uvicorn tonight. jfyk

Kludex avatar Sep 01 '22 10:09 Kludex

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

lovelydinosaur avatar Sep 01 '22 10:09 lovelydinosaur

This was merged on httpcore: https://github.com/encode/httpcore/pull/579

Kludex avatar Oct 30 '22 08:10 Kludex

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

Kludex avatar Oct 30 '22 09:10 Kludex