feature: Build schema on upstream version 3.9.0-rc5 #213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aiven-anton
commented
Oct 16, 2024
aiven-anton
force-pushed
the
aiven-anton/3.9.0
branch
2 times, most recently
from
7740b84
to
1244be0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 14 14
Lines 854 854
Branches 119 119
=========================================
Hits 854 854 ☔ View full report in Codecov by Sentry. |
aiven-anton
force-pushed
the
aiven-anton/3.9.0
branch
from
1244be0
to
209e7c8
Compare
aiven-anton
changed the title
aiven-anton
commented
Oct 29, 2024
aiven-anton
force-pushed
the
aiven-anton/3.9.0
branch
from
209e7c8
to
6615a91
Compare
Because UUID fields can always be None, the "optional" semantics around
them are special. Tagged fields also have special semantics, they can
always be omitted and are therefore by default also optional. With
3.9.0, the first field that combines both being UUID _and_ being a
tagged field was introduced, and it turns out we had buggy behavior for
this combination, resulting in generating code with an extra `| None`:
```python
replica_directory_id: uuid.UUID | None | None = field(
metadata={"kafka_type": "uuid", "tag": 0}, default=None
)
```
To accomodate for tagged UUID fields, where we have forced optionality for two reasons (all tagged fields are optional, all UUID fields are optional). We change the `get_reader()` implementation to not take optionality into account for UUID fields. This is in line with the corresponding `get_writer()` implementation. A somewhat more semantically correct option to this would be to not squash "is tag" and "is optional" into the same boolean value in `get_field_reader`, and rather keep track of that one level down. However, that would cause the already large combinatorial realm of possibilities to test for `get_reader()` to expand even larger. Living the semantically slightly worse, but much less complex option seems like the right thing to do.
See reasoning in #215
aiven-anton
force-pushed
the
aiven-anton/3.9.0
branch
from
6615a91
to
6055532
Compare
aiven-anton
changed the title
gpkc
approved these changes
Oct 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
@xfailto the compatibility test ofUpdateRaftVoterResponse. This model is hitting #215 which already exists prior to schema vertsion 3.9.0. That bug should be addressed separately.