You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "hachikuji (via GitHub)" <gi...@apache.org> on 2023/04/12 20:54:25 UTC
[GitHub] [kafka] hachikuji opened a new pull request, #13551: MINOR: Allow tagged fields with version subset of flexible version range
hachikuji opened a new pull request, #13551:
URL: https://github.com/apache/kafka/pull/13551
The generated message types are missing a range check for the case when the tagged version range is a subset of the flexible version range. This causes the tagged field count, which is computed correctly, to conflict with the number of tags serialized.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #13551: MINOR: Allow tagged fields with version subset of flexible version range
Posted by "hachikuji (via GitHub)" <gi...@apache.org>.
hachikuji commented on PR #13551:
URL: https://github.com/apache/kafka/pull/13551#issuecomment-1518292028
@cmccabe I think that case is already covered here: https://github.com/apache/kafka/blob/trunk/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java#L425. The case that is not covered is when the tagged range is a proper subset of the flexible version range.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #13551: MINOR: Allow tagged fields with version subset of flexible version range
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe commented on PR #13551:
URL: https://github.com/apache/kafka/pull/13551#issuecomment-1507673705
Thanks for finding this. I think the fix is sort of debatable. If a message has `flexibleVersions: "1+"` and we set `taggedVersions: "0+"` on some field, shouldn't that be an error that aborts processing the JSONs?
In the example above, I suspect that people casually scanning the file won't be aware that they're not really getting the tagged field on version 0. It's a gotcha, so we should just error out I think.
If you look at some of the stuff in `FieldSpec.checkTagInvariants`, we do the same thing. You can't tell the generator that something is a tagged field yet doesn't exist in some version, for example. You can't say that it's a tagged field but has no tag, etc. etc.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #13551: MINOR: Allow tagged fields with version subset of flexible version range
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe merged PR #13551:
URL: https://github.com/apache/kafka/pull/13551
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org