-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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
Remove getBrokerClusterAclConfig from admin and server #7486
Conversation
Why delete this method? How to obtain ACL configuration through the interface after deletion |
Codecov Report
@@ Coverage Diff @@
## develop #7486 +/- ##
=============================================
+ Coverage 42.93% 42.99% +0.05%
- Complexity 9660 9664 +4
=============================================
Files 1161 1158 -3
Lines 83930 83858 -72
Branches 10898 10889 -9
=============================================
+ Hits 36038 36053 +15
+ Misses 43410 43334 -76
+ Partials 4482 4471 -11
... and 20 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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
Suggested to check on the corresponding broker machine |
Yes, this is indeed possible. But I want to know, what are the reasons for doing this? Why delete it? thanks |
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
@@ -80,8 +80,6 @@ public class RequestCode { | |||
|
|||
public static final int UPDATE_GLOBAL_WHITE_ADDRS_CONFIG = 53; | |||
|
|||
public static final int GET_BROKER_CLUSTER_ACL_CONFIG = 54; |
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.
Should we reserve the request code to avoid duplication in the future?
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
Which Issue(s) This PR Fixes
Fixes #issue_id
Brief Description
How Did You Test This Change?