soarez commented on code in PR #14290:
URL: https://github.com/apache/kafka/pull/14290#discussion_r1345336375
##########
generator/src/main/java/org/apache/kafka/message/FieldSpec.java:
##########
@@ -157,11 +157,6 @@ private void checkTagInvariants() {
this.taggedVersions + ", which is not open-ended.
taggedVersions must " +
"be either none, or an open-ended range (that ends with a
plus sign).");
}
- if
(!this.taggedVersions.intersect(this.versions).equals(this.taggedVersions)) {
- throw new RuntimeException("Field " + name + " specifies
taggedVersions " +
- this.taggedVersions + ", and versions " + this.versions +
". " +
- "taggedVersions must be a subset of versions.");
- }
Review Comment:
We are not removing the tagged field. `"tag": 2` stays reserved to the
`Replicas` field, which prevents it from being reused. We're only closing the
`versions` range.
https://github.com/apache/kafka/pull/14290/files#diff-b157273749dc11f19aa59bd9ba179f9d9d36f22a30b2ec9aef6491aadb3f9942L33-R36
Closing the version range has desirable consequences:
* removes the field from the generated schema
* prevents serializing the field outside the version range
* generates a runtime error if deserializing the field outside the version
range
* marks deprecation of the field in the JSON definition
Even if a tagged field is no longer used we still need to keep the tag
reserved to prevent reuse, so the rule "taggedVersions must be a subset of
versions." enforced by the removed validation above doesn't seem to make sense?
Am I missing some downside to removing it?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
