Skip to content

Accept any sequence of header pairs, not just list - #204

Open
gyanu2507 wants to merge 3 commits into
python-hyper:mainfrom
gyanu2507:headers-as-sequence
Open

Accept any sequence of header pairs, not just list#204
gyanu2507 wants to merge 3 commits into
python-hyper:mainfrom
gyanu2507:headers-as-sequence

Conversation

@gyanu2507

Copy link
Copy Markdown

We only iterate header pairs. Typing them as list meant a tuple or h11.Headers needed a pointless list(...) copy.

Fixes #173

Callers shouldn't have to copy a tuple or h11 headers object into a list.
Comment thread src/wsproto/typing.py Outdated
@gyanu2507

Copy link
Copy Markdown
Author

@Kriechi switched the alias to collections.abc.Sequence in 3330d6d.

@gyanu2507

Copy link
Copy Markdown
Author

@Kriechi switched the alias to collections.abc.Sequence in 3330d6d. Please re-review when you can.

@gyanu2507
gyanu2507 requested a review from Kriechi August 31, 2026 13:14
@gyanu2507

Copy link
Copy Markdown
Author

CI is green on tests; the remaining mark is codecov/project coverage. No functional failure on the Sequence-of-headers change.

Tuple-backed header sequences exercise the list() conversions added when
Headers widened beyond list.
@gyanu2507

Copy link
Copy Markdown
Author

Pushed Sequence-backed header tests (tuple as well as list) for the list(headers) conversions. GitHub Actions is waiting on maintainer approval for the new run (action_required on the latest CI workflow). Once that runs, Codecov should refresh.

Comment thread src/wsproto/handshake.py
subprotocol = None
upgrade = b""
headers: Headers = []
headers: list[tuple[bytes, bytes]] = []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why can't it be the newly defined Headers type with a sequence here and in the similar code changes?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Those two locals are accumulators. We .append header pairs while walking h11's event. Headers is Sequence[tuple[bytes, bytes]], and mypy --strict then reports Sequence[...] has no attribute "append" on those two calls (lines 229 and 420).

Incoming values and extra_headers already use Headers. The handshake still builds a list, then stores it on Request.extra_headers.

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.

Type compatibility with h11._headers.Header

2 participants