Update types for h11 v0.13 #526

zanieb · 2022-03-25T16:08:58Z

Replaces #503, performing the additional work necessary for type checks to pass.

A cast is necessary because h11 does not expose the Sentinel type and uses it as a return type annotation instead of specifying the subtypes that can be returned.

event = cast(
    Union[h11.Event, h11.NEED_DATA, h11.PAUSED],
    self._h11_state.next_event(),
)

I've opened a patch to h11 to update next_event to have a return type that is narrower and a part of their public API: python-hyper/h11#144 — Note this is now merged resolving the need for a cast, but is not yet released.

There is currently no handling in _receive_event for the h11.PAUSED sentinel event. I am not sure what behavior you'd like to add there and have left a stub that should be filled in before this is merged. The documentation for PAUSED can be found at https://h11.readthedocs.io/en/latest/api.html#flow-control — Note this is not a concern and handling is performed here.

Note this partially addresses #509 by allowing a newer version of h11 to be used, but does not close the issue as it requests that the upper pin be removed entirely.

Closes #503
Closes #498

Allow for h11 V. 013. Facilitates installation alongside latest version of Uvicorn.

This also reverted commit f9a9847 which was dropped during rebase with the `master` branch. We probably do not want to suggest that passing `None` to the `write` function is proper. The short circuit there (return on empty buffer) should be targeted to an empty byte array.

…ling

GalaxySnail · 2022-04-09T08:24:31Z

I think it may not be a good idea to pin h11 under 0.14, it can cause unnecessary conflicts. See: https://hynek.me/articles/semver-will-not-save-you/

zanieb · 2022-04-09T18:11:59Z

I think it may not be a good idea to pin h11 under 0.14, it can cause unnecessary conflicts. See: https://hynek.me/articles/semver-will-not-save-you/

That sentiment is the point of issue #509. This pull request is to add compatibility which is necessary whether the pin is removed or not. There are other dependencies with upper versions in this project and that decision should be made separately from this fix.

graingert · 2022-06-19T16:28:24Z

httpcore/_async/http11.py

@@ -191,8 +195,11 @@ async def _receive_event(self, timeout: Optional[float] = None) -> H11Event:
                    raise RemoteProtocolError(msg)

                self._h11_state.receive_data(data)
+            elif event is h11.PAUSED:
+                # TODO: Implement handling for paused


h11.PAUSED is handled in https://github.com/encode/httpcore/pull/526/files#diff-f1329e0e83048035c0a643ab5f3c666ae6565cba6393111fc0dc1afc4f2dbaa4R168

graingert · 2022-06-19T16:28:49Z

httpcore/_sync/http11.py

@@ -191,8 +180,11 @@ def _receive_event(self, timeout: Optional[float] = None) -> H11Event:
                    raise RemoteProtocolError(msg)

                self._h11_state.receive_data(data)
+            elif event is h11.PAUSED:
+                # TODO: Implement handling for paused


paused is handled here https://github.com/encode/httpcore/pull/526/files#diff-7b355550eef6173e16a39fc2fd633b09e9589e707ed9d2792dbb3e205cbd7f0eR155

graingert · 2022-06-19T17:12:15Z

httpcore/_async/http11.py

+                event = cast(
+                    Union[h11.Event, h11.NEED_DATA, h11.PAUSED],
+                    self._h11_state.next_event(),
+                )

-            if event is h11.NEED_DATA:
+            if isinstance(event, h11.NEED_DATA):


I worry how much this cast and isinstance will slow stuff down - as this is a hot loop

we can hack around this by moving the cast outside the loop, but I'd want other maintainers to have a think on what's best here

httpcore/httpcore/_async/http11.py

Lines 30 to 49 in 627db6c

if TYPE_CHECKING:

from typing_extensions import Literal, Protocol, TypeAlias

class _Sentinel(enum.Enum):

PAUSED = enum.auto()

NEED_DATA = enum.auto()

_PausedType: TypeAlias = Literal[_Sentinel.PAUSED]

_NeedDataType: TypeAlias = Literal[_Sentinel.NEED_DATA]

_PAUSED: _PausedType = _Sentinel.PAUSED

_NEED_DATA: _NeedDataType = _Sentinel.NEED_DATA

class _NextEventType(Protocol):

async def __call__(self) -> Union[h11.Event, _PausedType, _NeedDataType]:

...

else:

_PausedType = _PAUSED = h11.PAUSED

_NeedDataType = _NEED_DATA = h11.NEED_DATA

_Sentinel = _NextEventType = object

httpcore/httpcore/_async/http11.py

Lines 195 to 207 in 627db6c

async def _receive_event(

self, timeout: Optional[float] = None

) -> Union[h11.Event, _PausedType]:

# The h11 type signature uses a private return type

next_event = cast(_NextEventType, self._h11_state.next_event)

while True:

with map_exceptions({h11.RemoteProtocolError: RemoteProtocolError}):

event = next_event()

if event is _NEED_DATA:

data = await self._network_stream.read(

self.READ_NUM_BYTES, timeout=timeout

)

graingert · 2022-06-19T17:24:07Z

httpcore/_sync/http11.py


    def _send_event(
-        self, event: H11Event, timeout: Optional[float] = None
+        self, event: h11.Event, timeout: Optional[float] = None


I think we still want our send_event to have a specific allowlist of events we send:

H11SendEvent = Union[ h11.Request, h11.Data, h11.EndOfMessage, ]

Suggested change

self, event: h11.Event, timeout: Optional[float] = None

self, event: H11SendEvent, timeout: Optional[float] = None

👍 makes sense to me. Seems like we would need this with the overload for correct typing.

graingert · 2022-06-19T17:27:36Z

httpcore/_sync/http11.py

    ) -> None:
        bytes_to_send = self._h11_state.send(event)
-        self._network_stream.write(bytes_to_send, timeout=timeout)
+        if bytes_to_send is not None:


we know we don't send ConnectionClosed so this check is redundant

send should be defined using overload

👍 I'll see if I can contribute that upstream

Oof, this would actually be a pretty big change upstream as the following is not a valid overload:

@overload def send(self, event: ConnectionClosed) -> None: ... @overload def send(self, event: Event) -> bytes: ... def send(self, event: Event) -> Optional[bytes]:

h11/_connection.py:505: error: Overloaded function signatures 1 and 2 overlap with incompatible return types [misc]

We'd need to introduce a separate base type (like Event) that indicates a null return.

I'm also exploring a generic

SendType = TypeVar('SendType', None, bytes) class Event(ABC, Generic[SendType]): ... class Foo(Event[bytes]): ... class ConnectionClosed(Event[None]): ...

but this would break type-checks for any downstream library that uses Event without a value for the generic e.g.

h11/_state.py:192: error: Missing type parameters for generic type "Event" [type-arg]

graingert · 2022-06-19T17:34:15Z

I think this is ready for review now @madkinsz

simonw · 2022-08-15T23:43:44Z

I'm seeing a weird intermittent bug with https://lite.datasette.io/ which it looks like would be resolved by merging this PR!

Intermittent error: Requested 'h11<0.13,>=0.11', but h11==0.13.0 is already installed simonw/datasette-lite#42

  File "/lib/python3.10/asyncio/tasks.py", line 232, in __step
    result = coro.send(None)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 310, in add_requirement
    return await self.add_requirement_inner(req)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 391, in add_requirement_inner
    if self.check_version_satisfied(req):
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 337, in check_version_satisfied
    raise ValueError(
ValueError: Requested 'h11<0.13,>=0.11', but h11==0.13.0 is already installed

It looks like something deep in the complex world of Pyodide may be installing h11==0.13 - even though I've tried in my code to ensure that h11==0.12 is installed first, here: https://github.com/simonw/datasette-lite/blob/d64455c2b7d89a32b8537c63f46db3579f0de91f/webworker.js#L53-L54

I'm going to keep on looking for a workaround, but I thought I'd drop a note here about this issue in case anyone else runs into a similar problem and lands here from a search.

zanieb · 2022-08-26T19:58:16Z

@graingert I'm not sure the remaining optimizations regarding the overload are feasible in the short term. It may be worth merging the current improvements to unblock upgrades and address the overload related checks once we can implement that upstream.

Can we get another maintainer to take a look at the hot-loop cast concerns?

zanieb · 2022-09-01T16:13:34Z

Note python-hyper/h11@04cc0f7 was merged a week ago so we can remove the aforementioned hot-loop cast once there's another h11 release.

Kludex · 2022-09-01T19:25:48Z

httpcore/_async/http11.py

+                event = cast(
+                    Union[h11.Event, h11.NEED_DATA, h11.PAUSED],
+                    self._h11_state.next_event(),
+                )


So I need to do this on uvicorn as well? 😞

I'm not sure if you do following release of python-hyper/h11#144 which updates that function's signature with narrowed return types.

eseglem · 2022-09-25T17:47:16Z

h11 just released v0.14.0. Does that mean this PR can remove the "hot loop cast" and move forward with another release for httpcore?

I have a server I deal with which behaves poorly and the h11 update will handle its behavior, so I look forward to having it make its way through h11 > httpcore > httpx releases.

zanieb · 2022-09-25T18:15:15Z

I'll look into updating this for h11 v0.14

Edit: See #579

tomchristie · 2022-09-26T11:17:27Z

Thanks @madkinsz - should we close this off in favour of #579?

zanieb · 2022-09-26T14:26:11Z

I think that's the best approach!

fsecada01 and others added 6 commits March 25, 2022 11:12

Update setup.py

7811c8b

Allow for h11 V. 013. Facilitates installation alongside latest version of Uvicorn.

Bump h11 pin to 0.14

6664449

Remove reassignment of event to prevent type failure

4207b8b

Replace h11 event with new base class type; add stubs for PAUSED hand…

ca728c2

…ling

Lint; add cast for return type

4de236c

zanieb force-pushed the h11-pin branch from ee8aa1c to 4de236c Compare March 25, 2022 16:18

zanieb mentioned this pull request Mar 25, 2022

Update next_event annotation to reflect possible return types python-hyper/h11#144

Merged

Merge branch 'master' into h11-pin

4347599

zanieb mentioned this pull request Apr 25, 2022

Consider dropping upper pin on h11 #509

Closed

yan12125 mentioned this pull request May 25, 2022

Dependency [h11<0.13.0] conflict while using alone with Hypercorn[h11==0.13.0] encode/httpx#2244

Closed

tomchristie and others added 2 commits May 25, 2022 13:47

Merge branch 'master' into h11-pin

ce52141

Merge branch 'master' into h11-pin

71cc46d

graingert reviewed Jun 19, 2022

View reviewed changes

graingert added 3 commits June 19, 2022 17:51

Merge branch 'master' into h11-pin

1ed1413

apply unasync

668dce8

do not attempt to handle paused in _receive_event

f42eab9

graingert reviewed Jun 19, 2022

View reviewed changes

graingert marked this pull request as ready for review June 19, 2022 17:34

Merge branch 'master' into h11-pin

2a10a07

simonw mentioned this pull request Aug 15, 2022

Intermittent error: Requested 'h11<0.13,>=0.11', but h11==0.13.0 is already installed simonw/datasette-lite#42

Closed

SergioSim mentioned this pull request Sep 1, 2022

✨(api) add xAPI statement forwarding background task openfun/ralph#199

Merged

1 task

Merge branch 'master' into h11-pin

13b733b

Merge branch 'master' into h11-pin

e3c6335

Kludex reviewed Sep 1, 2022

View reviewed changes

zanieb mentioned this pull request Sep 25, 2022

Update types for h11 v0.14 #579

Merged

zanieb closed this Sep 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update types for h11 v0.13 #526

Update types for h11 v0.13 #526

zanieb commented Mar 25, 2022 •

edited

GalaxySnail commented Apr 9, 2022

zanieb commented Apr 9, 2022

graingert Jun 19, 2022

graingert Jun 19, 2022

graingert Jun 19, 2022

graingert Jun 19, 2022 •

edited

graingert Jun 19, 2022 •

edited

zanieb Aug 24, 2022

graingert Jun 19, 2022

zanieb Aug 24, 2022

zanieb Aug 26, 2022 •

edited

zanieb Aug 26, 2022 •

edited

graingert commented Jun 19, 2022

simonw commented Aug 15, 2022 •

edited

zanieb commented Aug 26, 2022 •

edited

zanieb commented Sep 1, 2022

Kludex Sep 1, 2022

zanieb Sep 1, 2022

eseglem commented Sep 25, 2022

zanieb commented Sep 25, 2022 •

edited

tomchristie commented Sep 26, 2022

zanieb commented Sep 26, 2022

	if TYPE_CHECKING:
	from typing_extensions import Literal, Protocol, TypeAlias

	class _Sentinel(enum.Enum):
	PAUSED = enum.auto()
	NEED_DATA = enum.auto()

	_PausedType: TypeAlias = Literal[_Sentinel.PAUSED]
	_NeedDataType: TypeAlias = Literal[_Sentinel.NEED_DATA]
	_PAUSED: _PausedType = _Sentinel.PAUSED
	_NEED_DATA: _NeedDataType = _Sentinel.NEED_DATA

	class _NextEventType(Protocol):
	async def __call__(self) -> Union[h11.Event, _PausedType, _NeedDataType]:
	...

	else:
	_PausedType = _PAUSED = h11.PAUSED
	_NeedDataType = _NEED_DATA = h11.NEED_DATA
	_Sentinel = _NextEventType = object

	async def _receive_event(
	self, timeout: Optional[float] = None
	) -> Union[h11.Event, _PausedType]:
	# The h11 type signature uses a private return type
	next_event = cast(_NextEventType, self._h11_state.next_event)
	while True:
	with map_exceptions({h11.RemoteProtocolError: RemoteProtocolError}):
	event = next_event()

	if event is _NEED_DATA:
	data = await self._network_stream.read(
	self.READ_NUM_BYTES, timeout=timeout
	)

	self, event: h11.Event, timeout: Optional[float] = None
	self, event: H11SendEvent, timeout: Optional[float] = None

Update types for h11 v0.13 #526

Update types for h11 v0.13 #526

Conversation

zanieb commented Mar 25, 2022 • edited

GalaxySnail commented Apr 9, 2022

zanieb commented Apr 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graingert Jun 19, 2022 • edited

Choose a reason for hiding this comment

graingert Jun 19, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanieb Aug 26, 2022 • edited

Choose a reason for hiding this comment

zanieb Aug 26, 2022 • edited

Choose a reason for hiding this comment

graingert commented Jun 19, 2022

simonw commented Aug 15, 2022 • edited

zanieb commented Aug 26, 2022 • edited

zanieb commented Sep 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eseglem commented Sep 25, 2022

zanieb commented Sep 25, 2022 • edited

tomchristie commented Sep 26, 2022

zanieb commented Sep 26, 2022

zanieb commented Mar 25, 2022 •

edited

graingert Jun 19, 2022 •

edited

graingert Jun 19, 2022 •

edited

zanieb Aug 26, 2022 •

edited

zanieb Aug 26, 2022 •

edited

simonw commented Aug 15, 2022 •

edited

zanieb commented Aug 26, 2022 •

edited

zanieb commented Sep 25, 2022 •

edited