Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tagged struct fields with implicit defaults should be omitted #215

Closed
aiven-anton opened this issue Oct 25, 2024 · 0 comments · Fixed by #216
Closed

Tagged struct fields with implicit defaults should be omitted #215

aiven-anton opened this issue Oct 25, 2024 · 0 comments · Fixed by #216

Comments

@aiven-anton
Copy link
Collaborator

aiven-anton commented Oct 25, 2024

See discussion here.

For existing constructs like LeaderIdAndEpoch on FetchSnapshotResponse, we are not properly omitting them from serialization based on implicit default values. In this case, omitting the field should result in parsing into LeaderIdAndEpoch(leader_id=0, leader_epoch=0) and vice versa that value should result in an omitted tag.

aiven-anton added a commit that referenced this issue Oct 25, 2024
See reasoning in #215
@aiven-anton aiven-anton changed the title issue: New serialization semantics in UpdateRaftVoterResponse on 3.9.0 Tagged struct fields with implicit defaults should be omitted Oct 29, 2024
aiven-anton added a commit that referenced this issue Oct 29, 2024
See reasoning in #215
aiven-anton added a commit that referenced this issue Oct 29, 2024
See reasoning in #215
aiven-anton added a commit that referenced this issue Oct 29, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].

[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]: https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
aiven-anton added a commit that referenced this issue Oct 29, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].

[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]: https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
aiven-anton added a commit that referenced this issue Oct 29, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].

[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]: https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
aiven-anton added a commit that referenced this issue Oct 29, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].

[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]: https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
aiven-anton added a commit that referenced this issue Oct 29, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].

[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]: https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
github-merge-queue bot pushed a commit that referenced this issue Oct 30, 2024
This bumps schema to 3.9.0-rc5. The intention is that we can release
this as some pre-release, most likely an alpha, pending the upstream
final version.

This PR adds an `@xfail` to the compatibility test of
`UpdateRaftVoterResponse`. This model is hitting #215 which already
exists prior to schema vertsion 3.9.0. That bug should be addressed
separately.
aiven-anton added a commit that referenced this issue Oct 30, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].

[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]: https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
github-merge-queue bot pushed a commit that referenced this issue Nov 1, 2024
This fixes an issue where the logic for tagged fields was not in sync
with upstream. This is in the realm of undocumented features of the
Kafka protocol, so there is no accurate documentation to reference. The
closest to documentation that exists, is [this section in a source code
README][messages-readme].


[messages-readme]:https://github.com/apache/kafka/tree/4419677e06b77ed9103c179d713450b1eb7881a1/clients/src/main/resources/common/message#deserializing-messages

There was a [long discussion] about this behavior on the Kafka dev
mailing list.

[long discussion]:
https://www.mail-archive.com/dev@kafka.apache.org/msg144437.html

This fixes #215.
@gpkc gpkc closed this as completed in #216 Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant