kevin-wu24 commented on code in PR #22191:
URL: https://github.com/apache/kafka/pull/22191#discussion_r3290468845
##########
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:
This is also copied from the broker unregistration case.
Instead of dumping a stack trace in the case the cluster does not support
the MV for unregistering controllers, it prints a simple message to the user. I
think that is better UX for this case. I think we should also handle this in a
similar way for other "expected exceptions" like `InvalidRequestException`, and
`ControllerIdNotRegisteredException`. What do you think?
--
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]
