Skip to content

Update types for h11 v0.13 #526

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.

Sign up for GitHub

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

Closed
wants to merge 15 commits into from
Closed

Update types for h11 v0.13 #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7811c8b
Update setup.py
fsecada01 Feb 2, 2022
6664449
Bump h11 pin to 0.14
zanieb Mar 25, 2022
4207b8b
Remove reassignment of `event` to prevent type failure
zanieb Mar 25, 2022
3c46a89
Add an explicit check for `None` in `_send_event`
zanieb Mar 25, 2022
ca728c2
Replace h11 event with new base class type; add stubs for PAUSED hand…
zanieb Mar 25, 2022
4de236c
Lint; add cast for return type
zanieb Mar 25, 2022
4347599
Merge branch 'master' into h11-pin
zanieb Apr 9, 2022
ce52141
Merge branch 'master' into h11-pin
tomchristie May 25, 2022
71cc46d
Merge branch 'master' into h11-pin
graingert Jun 19, 2022
1ed1413
Merge branch 'master' into h11-pin
graingert Jun 19, 2022
668dce8
apply unasync
graingert Jun 19, 2022
f42eab9
do not attempt to handle paused in _receive_event
graingert Jun 19, 2022
2a10a07
Merge branch 'master' into h11-pin
tomchristie Jul 29, 2022
13b733b
Merge branch 'master' into h11-pin
tomchristie Sep 1, 2022
e3c6335
Merge branch 'master' into h11-pin
Kludex Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 23 additions & 17 deletions httpcore/_async/http11.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import enum
import time
from types import TracebackType
from typing import AsyncIterable, AsyncIterator, List, Optional, Tuple, Type, Union
from typing import (
AsyncIterable,
AsyncIterator,
List,
Optional,
Tuple,
Type,
Union,
cast,
)

import h11

Expand All @@ -17,15 +26,6 @@
from ..backends.base import AsyncNetworkStream
from .interfaces import AsyncConnectionInterface

H11Event = Union[
h11.Request,
h11.Response,
h11.InformationalResponse,
h11.Data,
h11.EndOfMessage,
h11.ConnectionClosed,
]


class HTTPConnectionState(enum.IntEnum):
NEW = 0
Expand Down Expand Up @@ -127,14 +127,14 @@ async def _send_request_body(self, request: Request) -> None:
event = h11.Data(data=chunk)
await self._send_event(event, timeout=timeout)

event = h11.EndOfMessage()
await self._send_event(event, timeout=timeout)
await self._send_event(h11.EndOfMessage(), timeout=timeout)

async def _send_event(
self, event: H11Event, timeout: Optional[float] = None
self, event: h11.Event, timeout: Optional[float] = None
) -> None:
bytes_to_send = self._h11_state.send(event)
await self._network_stream.write(bytes_to_send, timeout=timeout)
if bytes_to_send is not None:
await self._network_stream.write(bytes_to_send, timeout=timeout)

# Receiving the response...

Expand Down Expand Up @@ -168,12 +168,18 @@ async def _receive_response_body(self, request: Request) -> AsyncIterator[bytes]
elif isinstance(event, (h11.EndOfMessage, h11.PAUSED)):
break

async def _receive_event(self, timeout: Optional[float] = None) -> H11Event:
async def _receive_event(
self, timeout: Optional[float] = None
) -> Union[h11.Event, h11.PAUSED]:
while True:
with map_exceptions({h11.RemoteProtocolError: RemoteProtocolError}):
event = self._h11_state.next_event()
# The h11 type signature uses a private return type
event = cast(
Union[h11.Event, h11.NEED_DATA, h11.PAUSED],
self._h11_state.next_event(),
)
Comment on lines +177 to +180
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


if event is h11.NEED_DATA:
if isinstance(event, h11.NEED_DATA):
Comment on lines +177 to +182
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

@graingert graingert Jun 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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
)

data = await self._network_stream.read(
self.READ_NUM_BYTES, timeout=timeout
)
Expand Down
40 changes: 23 additions & 17 deletions httpcore/_sync/http11.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import enum
import time
from types import TracebackType
from typing import Iterable, Iterator, List, Optional, Tuple, Type, Union
from typing import (
Iterable,
Iterator,
List,
Optional,
Tuple,
Type,
Union,
cast,
)

import h11

Expand All @@ -17,15 +26,6 @@
from ..backends.base import NetworkStream
from .interfaces import ConnectionInterface

H11Event = Union[
h11.Request,
h11.Response,
h11.InformationalResponse,
h11.Data,
h11.EndOfMessage,
h11.ConnectionClosed,
]


class HTTPConnectionState(enum.IntEnum):
NEW = 0
Expand Down Expand Up @@ -127,14 +127,14 @@ def _send_request_body(self, request: Request) -> None:
event = h11.Data(data=chunk)
self._send_event(event, timeout=timeout)

event = h11.EndOfMessage()
self._send_event(event, timeout=timeout)
self._send_event(h11.EndOfMessage(), timeout=timeout)

def _send_event(
self, event: H11Event, timeout: Optional[float] = None
self, event: h11.Event, timeout: Optional[float] = None
Copy link
Member

@graingert graingert Jun 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

) -> 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:
Copy link
Member

Choose a reason for hiding this comment

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

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

send should be defined using overload

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I'll see if I can contribute that upstream

Copy link
Contributor Author

@zanieb zanieb Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@zanieb zanieb Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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]

self._network_stream.write(bytes_to_send, timeout=timeout)

# Receiving the response...

Expand Down Expand Up @@ -168,12 +168,18 @@ def _receive_response_body(self, request: Request) -> Iterator[bytes]:
elif isinstance(event, (h11.EndOfMessage, h11.PAUSED)):
break

def _receive_event(self, timeout: Optional[float] = None) -> H11Event:
def _receive_event(
self, timeout: Optional[float] = None
) -> Union[h11.Event, h11.PAUSED]:
while True:
with map_exceptions({h11.RemoteProtocolError: RemoteProtocolError}):
event = self._h11_state.next_event()
# The h11 type signature uses a private return type
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):
data = self._network_stream.read(
self.READ_NUM_BYTES, timeout=timeout
)
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def get_packages(package):
include_package_data=True,
zip_safe=False,
install_requires=[
"h11>=0.11,<0.13",
"h11>=0.11,<0.14",
"sniffio==1.*",
"anyio==3.*",
"certifi",
Expand Down