-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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-17915: Convert remaining Kafka Client system tests to use KRaft #18367
Conversation
A label of 'needs-attention' was automatically added to this PR in order to raise the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch @kevin-wu24! Overall lgtm. Couple of comments:
Regarding the quota_test, since is being migrated to kraft with this PR, should we revise the force_use_zk_connection
param it's still using?
Also, could you please run the updated test and share the output to validate the changes? Thanks!
Thanks for the review @lianetm!
Yes, this boolean is always
Link to system test run: https://semaphore.ci.confluent.io/workflows/6d93b69f-8d61-4b30-974b-09e092d276df passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kevin-wu24 ! LGTM.
Just for the record, here is the public link to the test results shared above: |
…#18367) Reviewers: Lianet Magrans <[email protected]>
Merged to trunk and cherry-picked to |
…apache#18367) Reviewers: Lianet Magrans <[email protected]>
Convert
tests/kafkatest/tests/client/client_compatibility_features_test.py
,tests/kafkatest/tests/client/client_compatibility_produce_consume_test.py
, andtests/kafkatest/tests/client/quota_test.py
to use KRaft.Committer Checklist (excluded from commit message)