Skip to content

Make Compression protocol public and default to gzip-only#103

Merged
anuraaga merged 5 commits intoconnectrpc:mainfrom
anuraaga:compression-rework
Jan 28, 2026
Merged

Make Compression protocol public and default to gzip-only#103
anuraaga merged 5 commits intoconnectrpc:mainfrom
anuraaga:compression-rework

Conversation

@anuraaga
Copy link
Collaborator

This reworks the interfaces related to compression to better align behavior with other connect implementations and provide more flexibility, and type safety

  • Makes Compression protocol public. Can be used to implement custom compressions
  • Accepts Compression instead of str throughout for type safety
  • Default compression is now gzip-only instead of using dependency detection. Compression implementations for zstd and brotli are provided in public API to allow registering them, possibly with custom levels, as users need
  • Send compression default to gzip instead of identity to match connect-go
  • Removes noextras project now that we don't do dependency detection. Yay

Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
@anuraaga anuraaga requested a review from a team January 27, 2026 08:56
Copy link
Member

@stefanvanburen stefanvanburen left a comment

Choose a reason for hiding this comment

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

👍, a nit and a question

from . import Compression


class GZipCompression(Compression):
Copy link
Member

Choose a reason for hiding this comment

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

minorest of nits: I'd prefer Gzip to GZip here, to match the stdlib.

interceptors: Iterable[Interceptor] = (),
read_max_bytes: int | None = None,
compressions: Iterable[str] | None = None,
compressions: Sequence[Compression] | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

I might have just missed it reading through, but why do we need a Sequence instead of an Iterable here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for calling this out - in a different project I got feedback that Sequence could make sense for an ordered list, i.e. set doesn't work then. But on a second thought, I know that such a parameter would be Iterable in Java so what works there should work here and switched back to it. If a user passes a set anyways, they probably have a strong reason for it for better or worse

Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
@anuraaga anuraaga merged commit c2844cb into connectrpc:main Jan 28, 2026
23 checks passed
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.

2 participants