Skip to content

Resolve request compression names in one place - #368

Draft
i2y wants to merge 1 commit into
mainfrom
resolve-request-compression
Draft

i2y wants to merge 1 commit into
mainfrom
resolve-request-compression

Conversation

@i2y

@i2y i2y commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Each server path looked up the request compression on its own: Connect unary POST and GET in both _server_async.py and _server_sync.py, plus negotiate_stream_compression in _protocol_connect.py and _protocol_grpc.py. The copies drifted apart, and #365 fixed three cases of it: an unknown Connect stream compression was treated as identity, Connect unary POST lowercased the name, and only some paths accepted an empty encoding.

This moves the lookup into resolve_request_compression in _compression.py, next to unknown_compression_error. It treats an empty name as identity, matches names exactly, and returns None for an unknown name, so callers still raise unknown_compression_error where they did before. There's no behavior change: the tests from #365 pass unchanged, and a small unit test covers the helper. uv run poe check and the server conformance suites pass.

This doesn't add conflicts with #361 beyond the ones it already has with main. The trio path in #348 goes through negotiate_stream_compression, so it picks this up as is.

Base automatically changed from reject-unknown-connect-stream-compression to main September 27, 2026 13:04
Each server path looked up the request compression on its own, and the
copies drifted apart: #365 fixed an unknown Connect stream compression
being treated as identity, Connect unary POST lowercasing the name, and
empty encodings being accepted on some paths only. Resolve the name in
resolve_request_compression instead, so every path treats an empty name
as identity and matches names exactly. No behavior change.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: i2y <6240399+i2y@users.noreply.github.com>
@i2y
i2y force-pushed the resolve-request-compression branch from 2f647a8 to a02f71a Compare September 27, 2026 13:05

This branch has not been deployed

No deployments
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.

1 participant