You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/08/03 10:27:44 UTC
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1285: HDDS-4056. Refactor WithScmClient to mixin
adoroszlai opened a new pull request #1285:
URL: https://github.com/apache/hadoop-ozone/pull/1285
## What changes were proposed in this pull request?
1. Move `--scm` parameter from `ozone admin` command to the subcommands that actually need it. Currently all except `OMAdmin` do, but this is likely to change by adding more `om` and other non-`scm` subcommands in the future. Now it will be possible to convert `OzoneAdmin` to the pluggable model in HDDS-4046, since it no longer depends on SCM functionality. This is made easier by:
* extracting `--scm`-related logic to the new `ScmOption` mixin,
* extracting abstract parent class for `ScmClient`-based subcommands (similar to `Handler` for `ozone shell` subcommands).
2. Upgrade picocli to 4.4.0. (Needed for `MIXEE`, but useful anyway.)
Note that moving the `--scm` parameter is backwards incompatible. Previously the following command was valid:
```
ozone admin --scm scm:9860 pipeline list
```
and this one was not:
```
ozone admin pipeline list --scm scm:9860
```
but now it's the other way around. If necessary, we could provide backwards compatibility, but I don't think it's worth the effort and extra code complexity.
https://issues.apache.org/jira/browse/HDDS-4056
## How was this patch tested?
Existing acceptance tests cover some of the commands. Also added few new test cases for `ozone admin pipeline`.
https://github.com/adoroszlai/hadoop-ozone/runs/933098595
https://github.com/adoroszlai/hadoop-ozone/runs/933098617
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek merged pull request #1285: HDDS-4056. Convert OzoneAdmin to pluggable model
Posted by GitBox <gi...@apache.org>.
elek merged pull request #1285:
URL: https://github.com/apache/hadoop-ozone/pull/1285
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1285: HDDS-4056. Convert OzoneAdmin to pluggable model
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1285:
URL: https://github.com/apache/hadoop-ozone/pull/1285#issuecomment-681825379
Thanks @elek for reviewing and committing it.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on a change in pull request #1285: HDDS-4056. Convert OzoneAdmin to pluggable model
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1285:
URL: https://github.com/apache/hadoop-ozone/pull/1285#discussion_r478253112
##########
File path: hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ScmOption.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hdds.HddsUtils;
+import org.apache.hadoop.hdds.cli.GenericParentCommand;
+import org.apache.hadoop.hdds.conf.MutableConfigurationSource;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY;
+import static picocli.CommandLine.Spec.Target.MIXEE;
+
+/**
+ * Defines command-line option for SCM address.
+ */
+public class ScmOption {
+
+ @CommandLine.Spec(MIXEE)
+ private CommandLine.Model.CommandSpec spec;
Review comment:
Today I learned it. Nice. Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #1285: HDDS-4056. Convert OzoneAdmin to pluggable model
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1285:
URL: https://github.com/apache/hadoop-ozone/pull/1285#issuecomment-681799441
```
ozone admin pipeline list --scm scm:9860
```
Personally I feel it more natural. Moving some parameters before the last subcommand is strange syntax for me. (= I think the new one is easier to use)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org