CASSANDRA-20876: Offline nodetool commands should not print network options in help#4871
CASSANDRA-20876: Offline nodetool commands should not print network options in help#4871arvindKandpal-ksolves wants to merge 1 commit into
Conversation
…ptions in help - Added isOfflineCommand() to AbstractCommand to publicly expose whether a command requires a JMX connection - Updated CassandraCliHelpLayout to suppress JmxConnect options in help output for offline commands - Updated help snapshot for 'history' to reflect the correct output
| { | ||
| try | ||
| { | ||
| return !shouldConnect(); |
There was a problem hiding this comment.
@arvindKandpal-ksolves The problem with the current approach is exactly that isOfflineCommand() calls shouldConnect(), which is an execution-flow method, intentionally runtim-overridable and side-effecting (that's why it declares throws ExecutionException). See the Sjk (where the 'locality' is runtime dependent) command and check the test NodetoolHelpCommandsOutputTest.
As for the history command we know in advance that it is local, and this knowledge should be a part of command metdata.
So the right fix is to make this knowledge to be a part of command hierary: so either create a LocalCommand marker interface, or AbstractLocalCommand extends AbstractCommand. Then in the Layout class, check whether the command is local by examining the class it extends.
2 participants
Summary
Offline nodetool commands (e.g.
history) do not require a JMX connection,yet their
--helpoutput was showing network-related options like--host,--port,--password, etc. This is misleading and unnecessary.Changes
AbstractCommand.java: Added a publicisOfflineCommand()method thatsafely wraps the existing
protected shouldConnect()to expose whether acommand requires a JMX connection.
CassandraCliHelpLayout.java: UpdatedparentCommandOptionsWithJmxOptions()to suppress
JmxConnectoptions from help output when the command is offline.NodetoolCommandoptions are still always shown for backwards compatibility.test/resources/nodetool/help/history: Updated help snapshot to reflectthe corrected output — JMX network options are no longer shown for
history.patch by Arvind Kandpal; reviewed by TBD for CASSANDRA-20876