ahuang98 commented on code in PR #22191:
URL: https://github.com/apache/kafka/pull/22191#discussion_r3291467245
##########
tools/src/main/java/org/apache/kafka/tools/ClusterTool.java:
##########
@@ -169,6 +182,20 @@ static void unregisterCommand(PrintStream stream, Admin
adminClient, int id) thr
}
}
+ static void unregisterControllerCommand(PrintStream stream, Admin
adminClient, int id) throws Exception {
+ try {
+ adminClient.unregisterController(id).all().get();
+ stream.println("Controller " + id + " is no longer registered.");
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof UnsupportedVersionException) {
+ stream.println("The target cluster does not support the
controller unregistration API.");
Review Comment:
my comment applies to the broker unregistration case too, the original
exception messages feel sufficient (and are more actionable in that they
specifically mention MV is insufficient) so it feels odd we don't just include
that in the output here. in any case, we don't have to change existing behavior
in this PR and we can just have the controller unregistration API match
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
