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

KAFKA-13385: In the KRPC request header, translate null clientID to empty #11385

Merged
merged 2 commits into from
Oct 27, 2021

Conversation

cmccabe
Copy link
Contributor

@cmccabe cmccabe commented Oct 8, 2021

Due to a historical quirk, the clientId field is nullable in the request header schema. However, the code does
not handle null clientIds. They should be treated as the empty string, as they were previously.

@cmccabe
Copy link
Contributor Author

cmccabe commented Oct 9, 2021

retest this please

@cmccabe cmccabe changed the title Translate null client IDs to the empty string KAFKA-13385: In the KRPC request header, translate null clientID to empty Oct 19, 2021
@dajac
Copy link
Contributor

dajac commented Oct 23, 2021

Good catch. Could we add a test to avoid this regression in the future?

@cmccabe
Copy link
Contributor Author

cmccabe commented Oct 27, 2021

Good catch. Could we add a test to avoid this regression in the future?

Yes, absolutely. I actually posted this without a test only to try to find out which junit test would fail when I changed the null handling. Unfortunately, it seems like none of them did! (Or rather, the only test failures I see in this run are unrelated test flakes.)

I'll add a unit test for this case.

Copy link
Contributor

@splett2 splett2 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

Copy link
Contributor

@dajac dajac left a comment

Choose a reason for hiding this comment

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

LGTM

@cmccabe cmccabe merged commit cf2b393 into apache:trunk Oct 27, 2021
@cmccabe cmccabe deleted the clientid branch October 27, 2021 22:19
xdgrulez pushed a commit to xdgrulez/kafka that referenced this pull request Dec 22, 2021
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 this pull request may close these issues.

4 participants