search

tychedelia / kafka-protocol-rs

Rust Kafka protocol
Apache License 2.0
63 stars 23 forks source link

Consider ignoring fields instead of failing to encode #85

Closed eblocha closed 2 months ago

eblocha commented 2 months ago

Currently the library will fail encoding if fields are set on requests when those fields are only relevant to newer api versions.

For example: https://github.com/tychedelia/kafka-protocol-rs/blob/main/src/messages/metadata_request.rs#L108

I am working on a Kafka client that can dynamically select a version based on the broker version range. Since the request will fail to encode based on the version selected, the creator of the message needs to know the version before creating it.

However, the client I am working on lazily connects and resolves versions only when a request is sent, so this version information is not available at creation time.

Could the encoders ignore fields that aren't relevant to the version being encoded?

tychedelia commented 2 months ago

Maybe we could explore adding this is a configurable option, but it definitely wouldn't be the correct default behavior. If you want to fork the library, this should be pretty easy to replace yourself, although I have to admit I'm a bit skeptical about your ability to generalize this approach... new protocol fields are semantically meaningful relative to the expected behavior of the broker, so at some point, the caller does indeed need to know the version of the broker they're interacting with to accomplish anything useful.

eblocha commented 2 months ago

Fair enough. It looks like the official Java client requires the version to create messages (and throws if incompatible), so this is equivalent.

I think I can change the approach I'm taking to acquire a connection first before creating the message.