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

fix: use version 4 of DescribeGroupsRequest only if kafka broker vers… …ion is >= 2.4 #2451

Merged

Conversation

faillefer
Copy link
Contributor

Fixes issue #2443

@faillefer faillefer force-pushed the bugfix/2443-fix-DescribeGroupsRequest-version branch from f1af2e4 to 8642dec Compare March 6, 2023 10:17
@dnwe
Copy link
Collaborator

dnwe commented Mar 7, 2023

@faillefer thanks, we'd also need to update this check here and here for the request + response required versions, which seems to be completely wrong at the moment.

The DescribeGroups protocol is interesting as the request format doesn't change very much between versions, but the response does. We should be supporting and submitting this set of versions:

1.1 2.0 2.1 2.2 2.3 2.4
DescribeGroups v1 v2 v2 v2 v3 v4*
  • we should really be v5 on 2.4+, but to do that we'd need to implement compact and tagged field on this protocol and we haven't done that yet — v4 is the highest version that doesn't require that

@dnwe dnwe closed this Mar 7, 2023
@dnwe dnwe reopened this Mar 7, 2023
@faillefer
Copy link
Contributor Author

I have updated both files. It's unclear to me if those versions apply both on Request and Response. I've checked the code in the kafka repository and it seems to be the case for v3 and v4 but i have no idea if it's also the case for other releases

@dnwe dnwe added the fix label Mar 7, 2023
Copy link
Collaborator

@dnwe dnwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@dnwe dnwe merged commit 7dbf0b5 into IBM:main Mar 7, 2023
@dnwe dnwe changed the title #2443 Use version 4 of DescribeGroupsRequest only if kafka broker version is > 2.4.0.0 fix: use version 4 of DescribeGroupsRequest only if kafka broker vers… …ion is >= 2.4 Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants