Move CompressionAlgorithm to core #1858
Conversation
Motivation: `CallOptions` only let you enable/disable compression, it doesn't allow you to control which algorithm should be used. This is an unnecessary limitation. This was done because `CompressionAlgorithm` lives in the http2 module. Modifications: - Move `CompressionAlgorithm` to the core module - Rename 'identity' to 'none' as that's clearer for users - Add extensions in the http2 module to create an algorithm from its name - Add a `CompressionAlgorithmSet` type which uses an option set which allows for cheaper updates. - Update call options Result: - `CallOptions` is more flexible - Updating the call options set is cheaper
| /// The algorithm used for compressing outbound messages. | ||
| /// | ||
| /// Note that this option is _advisory_: transports are not required to support compression. | ||
| public var requests: Bool | ||
| /// If `nil` the value configured on the transport will be used instead. | ||
| public var algorithm: CompressionAlgorithm? | ||
|
|
||
| /// Whether response messages are permitted to be compressed. | ||
| public var responses: Bool | ||
| /// The enabled compression algorithms. | ||
| /// | ||
| /// If `nil` the value configured on the transport will be used instead. | ||
| public var enabledAlgorithms: CompressionAlgorithmSet? |
There was a problem hiding this comment.
I find this naming a bit unclear, because it's not obvious that algorithm is used for requests, and enabledAlgorithms for responses. Can we use something like outboundAlgorithm and supportedInboundAlgorithms instead?
There was a problem hiding this comment.
That's fair. My aim was to make this API simpler but don't think I achieved that with the current semantics.
I looked into other implementations some more and they don't expose the enabled set at the call level:
- Java:
withCompression - Python:
compression=... - C++:
set_compression_algorithm
They also use the set of algorithms to validate that the selected algorithm for requests is allowed, i.e. if you set the enabled algorithms to be "gzip" and "none" on the transport, and select "deflate" at the call level, then the request won't be compressed, which makes sense.
So I think what I'm proposing here is to reframe "enabled response algorithms" from "algorithms we support for decompressing responses" to "algorithms we support". More concretely:
- remove
enabledAlgorithmsfrom the call level, and only allow it to be set at transport creation time - keep
algorithmand note that it may not be applied if it isn't enabled on the transport - have a default compression algorithm config on the transport
This also puts it inline with other implementations. WDYT?
There was a problem hiding this comment.
Sounds good, thanks for doing the research.
| } | ||
|
|
||
| extension CompressionAlgorithmSet { | ||
| /// A sequence of ``CompressionAlgorithm`` values present in the set.. |
There was a problem hiding this comment.
| /// A sequence of ``CompressionAlgorithm`` values present in the set.. | |
| /// A sequence of ``CompressionAlgorithm`` values present in the set. |
2 participants
Motivation:
CallOptionsonly let you enable/disable compression, it doesn't allow you to control which algorithm should be used. This is an unnecessary limitation. This was done becauseCompressionAlgorithmlives in the http2 module.Modifications:
CompressionAlgorithmto the core moduleCompressionAlgorithmSettype which uses an option set which allows for cheaper updates.Result:
CallOptionsis more flexible