feat: add isValidVersion to protocol types #2538
Merged
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
dnwe
force-pushed
the
dnwe/is-valid-version
branch
from
f76c25b
to
26d9b01
Compare
hindessm
reviewed
Aug 1, 2023
hindessm
reviewed
Aug 1, 2023
hindessm
reviewed
Aug 1, 2023
dnwe
force-pushed
the
dnwe/is-valid-version
branch
3 times, most recently
from
ca24f79
to
70d2c11
Compare
hindessm
reviewed
Aug 1, 2023
dnwe
force-pushed
the
dnwe/is-valid-version
branch
3 times, most recently
from
12e0507
to
47c6581
Compare
The intention here is that we can check that a request version value is supported by the protocol encoder/decoder before sending it Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
This is identical to v0 but can used from broker 2.0.0.0 onwards Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
dnwe
force-pushed
the
dnwe/is-valid-version
branch
from
47c6581
to
7b96841
Compare
Testing uncovered a few mismatches between the requiredVersion implementation in the request and response for these types Also fix wrong key() in alter_configs_response! Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
Co-authored-by: Mark Hindess <hindessm@users.noreply.github.com> Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
This is really just proxying to FindCoordinatorRequest/FindCoordinatorResponse, but for now just copy in the same isValidVersion/requiredVersion code and ensure we're passing Version to and from it correctly. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
Initially seeded with only the protocol versions required for Kafka v1.1, check we've implemented the expected versions and they pass the isValidVersion and the requiredVersion checks as expected. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
dnwe
force-pushed
the
dnwe/is-valid-version
branch
from
7b96841
to
b8cc2b1
Compare
Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
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.
The intention here is to add a common
isValidVersionto all the protocol types so that we can check that a request version value is supported by the protocol encoder/decoder implementation before sending it (hence avoiding malformed requests on the broker side).Additionally in this PR fixup the existing
requiredVersionchecks where they were incorrect and/or add missing values, also ensuring the request and response implementations were in agreement.Finally add some testing in this area and in a few places start using newer version numbers where the config version value allows.
Contributes-to: #2408