[GitHub] [hadoop-ozone] elek commented on a change in pull request #687: HDDS-2184. Rename ozone scmcli to ozone admin

2020-03-25 Thread GitBox
elek commented on a change in pull request #687: HDDS-2184. Rename ozone scmcli 
to ozone admin
URL: https://github.com/apache/hadoop-ozone/pull/687#discussion_r397878003
 
 

 ##
 File path: hadoop-hdds/docs/content/tools/SCMCLI.md
 ##
 @@ -1,27 +0,0 @@

 
 Review comment:
   I deleted the old file because it didn't have any useful information. But I 
agree, it can be a good idea to create new documentation.
   
   I created a short, initial version : I documented all the subcommand groups 
to make it clear what functionalities are available, but didn't specify all the 
details/flags as they can be get with `--help` easily.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] elek commented on a change in pull request #687: HDDS-2184. Rename ozone scmcli to ozone admin

2020-03-25 Thread GitBox
elek commented on a change in pull request #687: HDDS-2184. Rename ozone scmcli 
to ozone admin
URL: https://github.com/apache/hadoop-ozone/pull/687#discussion_r397875948
 
 

 ##
 File path: 
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCommands.java
 ##
 @@ -46,15 +48,15 @@
   LoggerFactory.getLogger(SafeModeCommands.class);
 
   @ParentCommand
-  private SCMCLI parent;
+  private WithScmClient parent;
 
-  public SCMCLI getParent() {
+  public WithScmClient getParent() {
 return parent;
   }
 
   @Override
   public Void call() throws Exception {
 throw new MissingSubcommandException(
-this.parent.getCmd().getSubcommands().get("safemode"));
+new CommandLine(new SafeModeCommands()));
 
 Review comment:
   I agree with you about the usability benefit, but I deleted it 
intentionally. I don't like that we have a strong dependency on the parent 
command (getCmd() should be available) and it was not easy to keep it as from 
now, we have subcommands in multiple subprojects (eg. ozone-tools, hdds-tools)
   
   Fortunately I found an other way to get the desired output: It runed out 
that it's enough to have a `CommandSpec` reference which can be injected with 
`@Spec`. I updated the subcommand, and it works again:
   
   ```
   Incomplete command
   Usage: ozone admin safemode [-hV] [COMMAND]
   Safe mode specific operations
 -h, --help  Show this help message and exit.
 -V, --version   Print version information and exit.
   ```


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] elek commented on a change in pull request #687: HDDS-2184. Rename ozone scmcli to ozone admin

2020-03-25 Thread GitBox
elek commented on a change in pull request #687: HDDS-2184. Rename ozone scmcli 
to ozone admin
URL: https://github.com/apache/hadoop-ozone/pull/687#discussion_r397824173
 
 

 ##
 File path: 
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/WithScmClient.java
 ##
 @@ -0,0 +1,29 @@
+/**
+ * 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.container;
+
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+
+/**
+ * Command which provides and SCM client based on the current config.
+ */
+public interface WithScmClient {
 
 Review comment:
   Based on the function, this interface nothing more just providing an SCM 
client. Yes, I agree that it's used by admin commands but I am not sure if it's 
important here. (It can be used for any other command, eg. with Freon)


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org