sodonnel commented on a change in pull request #3179:
URL: https://github.com/apache/ozone/pull/3179#discussion_r825900113



##########
File path: 
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java
##########
@@ -90,8 +99,17 @@ private void outputContainerInfo(ContainerInfo containerInfo)
 
   @Override
   public void execute(ScmClient scmClient) throws IOException {
+    if (!Strings.isNullOrEmpty(replication) && type == null) {
+      throw new IOException("Type must be set if replication is passed");
+    }

Review comment:
       I made this change - it stops it bailing out, but we can get a more ugly 
error if you pass an EC replication string now without type:
   
   ```
   bash-4.2$ ozone admin container list --replication THREE
   {
     "state" : "OPEN",
     "replicationConfig" : {
       "replicationFactor" : "THREE",
       "replicationType" : "RATIS"
     },
     "usedBytes" : 0,
     "numberOfKeys" : 0,
     "lastUsed" : "2022-03-14T12:32:04.946Z",
     "stateEnterTime" : "2022-03-14T12:30:50.776Z",
     "owner" : "om1",
     "containerID" : 3,
     "deleteTransactionId" : 0,
     "sequenceId" : 0,
     "open" : true
   }
   bash-4.2$ ozone admin container list --replication rs-3-2-1024k
   No enum constant org.apache.hadoop.hdds.client.ReplicationFactor.rs-3-2-1024k
   ```
   I'd like to avoid parsing / checking replication strings in these commands 
incase things change in the future. Eg, I could look for replication in (ONE, 
THREE) and then guess RATIS if `type` is not present, but if we added some 
other factor later (FIVE or TWO) it would require a change here, and possibly 
in other commands scattered around.
   
   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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to