Tejaskriya commented on code in PR #8674:
URL: https://github.com/apache/ozone/pull/8674#discussion_r2191549940
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DecommissionStatusSubCommand.java:
##########
@@ -51,35 +51,36 @@ public class DecommissionStatusSubCommand extends
ScmSubcommand {
private String errorMessage = "Error getting pipeline and container metrics
for ";
- @CommandLine.Option(names = { "--id" },
- description = "Show info by datanode UUID",
- defaultValue = "")
- private String uuid;
-
- @CommandLine.Option(names = { "--ip" },
- description = "Show info by datanode ipAddress",
- defaultValue = "")
- private String ipAddress;
-
@CommandLine.Option(names = { "--json" },
description = "Show output in json format",
defaultValue = "false")
private boolean json;
+ @CommandLine.Mixin
+ private NodeSelectionMixin nodeSelectionMixin;
+
+ @CommandLine.Spec
+ private CommandLine.Model.CommandSpec spec;
+
@Override
public void execute(ScmClient scmClient) throws IOException {
+ if (!nodeSelectionMixin.getHostname().isEmpty()) {
+ throw new CommandLine.ParameterException(spec.commandLine(),
+ "--hostname option not supported for this command");
Review Comment:
Although this logic preserves the command execution, I think if we try to
check the --help message for the command, we would still see the hostname
option coming up right? This would be misleading.
Perhaps we can mention in the description of the option that it is not
supported for the decommission status command as this is the only exception for
now.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/NodeSelectionMixin.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.cli.datanode;
+
+import com.google.common.base.Strings;
+import picocli.CommandLine;
+
+/**
+ * Picocli mixin providing standardized datanode selection options for
consistent CLI usage across commands.
+ */
+public class NodeSelectionMixin {
+
+ @CommandLine.ArgGroup(exclusive = true, multiplicity = "0..1")
+ private Selection selection = new Selection();
+
+ //Precedence order: --node-id > --id (deprecated) > --uuid (deprecated).
+ public String getNodeId() {
+ return !Strings.isNullOrEmpty(selection.nodeId) ? selection.nodeId :
+ !Strings.isNullOrEmpty(selection.id) ? selection.id :
+ !Strings.isNullOrEmpty(selection.uuid) ? selection.uuid : "";
+ }
+
+ public String getHostname() {
+ return selection.hostname;
+ }
+
+ public String getIp() {
+ return selection.ip;
+ }
+
+ static class Selection {
+
+ @CommandLine.Option(names = "--node-id", description = "Show info by
datanode UUID.", defaultValue = "")
Review Comment:
Let's keep the description consistent between the various options in the
mixin.
```suggestion
@CommandLine.Option(names = "--node-id", description = "UUID of the
datanode.", defaultValue = "")
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
