-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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-14582: Move JmxTool to tools #13136
Conversation
2e97c4a
to
16b90df
Compare
1b927c6
to
b7daf79
Compare
@mimaison @clolov @vamossagar12 this is ready for review if you have some time. |
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 @fvaleri for the PR. I left a few comments. I think we should use this opportunity to add a few tests too. Ideally write them against the previous tool first to ensure the new version works the same way.
28d9f91
to
cbdb6ea
Compare
Thanks @mimaison. I addressed all your comments and now working on the test suite. |
…lasses These classes are required by most commands, so they must be migrated first. Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
f05b104
to
37b998a
Compare
@mimaison I've rebased and added some tests. This is now ready for another review. Thanks. |
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
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 PR. I left a few questions.
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
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.
LGTM
Looks like this caused some system test failures: http://confluent-kafka-system-test-results.s3-us-west-2.amazonaws.com/trunk/2023-02-11--001.system-test-kafka-trunk--1676126329--confluentinc--master--d0d9e3f297/report.html
|
This PR is based on #13131.
Output example:
System test using it: