kafka-protocol-rs
kafka-protocol-rs copied to clipboard
allow easily using multiple versions of ListOffsets api
this code actually prevents writing user code that works across multiple versions.
for example, if i want to support both new and latest version of ListOffsets api, i would just set the leader_epoch field in the respective Builder class and if the api version during encoding is <4, the encoder will simply ignore that field.
currently due to this bail, i have to add if checks everywhere in my code to ensure the right fields are set
The code you have changed here is auto-generated, to actually change this code you need to change the code generator, otherwise it'll be blown away next time we rerun the generator.
But more importantly, I think these generated asserts are valuable and I don't think they should be removed. As you've observed, to use this API with the assert we need to add some kind of check to our values:
let leader_epoch = if leader_epoch_supported {
leader_epoch
} else {
-1
};
ListOffsetsPartitionResponse::default().with_leader_epoch(leader_epoch)
There are two reasons I see this as a good thing:
- At development time: It reminds us to consider the consequences of this value not being available on older protocol versions, maybe we need to alter other parts of our logic as well when the field is not available.
- At maintenance/debugging time: It makes it obvious at a glance that the client will not receive this field on this version of the protocol.
For example in this case the broker should not act under the assumption that the client knows the leader_epoch for protocol versions where its not supported. I'm not sure that is actually a problem faced when implementing a kafka compatible server, but it demonstrates the kind of issue I'm trying to protect against at least.
Cllosing as per the reasoning in my previous comment. Feel free to reopen the discussion.