dajac commented on PR #18261: URL: https://github.com/apache/kafka/pull/18261#issuecomment-2552986271
> I'm not convinced by the OffsetCommitV0 thing. I think something you're relying on and benefitting from the fact that OffsetCommitV0 v0 is exactly the same as OffsetCommit v0. Sometimes, you have to convert from the v0 class to the current one. Just seems a bit of a faff. Can the definition of the coordinator-key schemas be flexible enough to allow just OffsetCommit to exist without the separate V0 schema? I am not really happy with the OffsetCommit version 0. I considered extending the schema to support a legacy api key but it brings even more complexity. The goal of this refactoring to introduce the enum with all the types and their versions and to have strong semantic verified for the records. Having alternate api keys, makes this part harder. We could make an exception as you suggest but then it does not fit nicely in the enum either. Hence, I went with the separate record. I would argue that the separate record is not wrong because we actually changed the record type but we kept the same name. I think that we introduced this awkwardness when we transitioned to using the auto-generated records. We could perhaps rename that record to `LegacyOffsetCommit` to make the separation clearer. By the way, that legacy record was only used in Apache Kafka 0.8. It is very unlikely to ever see it in 4.0 clusters. I was also considering to just drop it but we would need a small KIP for it. It is too bad that the keep freeze for 4.0 is passed because I suppose that we will need to wait for 5.0 until we can remove it now. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
