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/09/01 23:08:27 UTC

[GitHub] [hadoop-ozone] amaliujia opened a new pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

amaliujia opened a new pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375


   
   ## What changes were proposed in this pull request?
   
   Change `ozone admin om getserviceroles` to `ozone admin om status`
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4189
   
   ## How was this patch tested?
   
   Unit Test


----------------------------------------------------------------
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] amaliujia commented on pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#issuecomment-685181181


   R: @adoroszlai @timmylicheng 


----------------------------------------------------------------
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] cxorm commented on a change in pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
cxorm commented on a change in pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#discussion_r481576976



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/GetServiceRolesSubcommand.java
##########
@@ -30,10 +30,10 @@
 import java.util.concurrent.Callable;
 
 /**
- * Handler of om get-service-roles command.
+ * Handler of om status command.
  */
 @CommandLine.Command(
-    name = "getserviceroles",
+    name = "status",

Review comment:
       Thanks @adoroszlai for the idea, I agree with 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] amaliujia commented on a change in pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#discussion_r481610070



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/GetServiceRolesSubcommand.java
##########
@@ -30,10 +30,10 @@
 import java.util.concurrent.Callable;
 
 /**
- * Handler of om get-service-roles command.
+ * Handler of om status command.
  */
 @CommandLine.Command(
-    name = "getserviceroles",
+    name = "status",

Review comment:
       That makes sense. Suggestion applied.




----------------------------------------------------------------
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] amaliujia commented on pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om roles`

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#issuecomment-686137691


   @adoroszlai 
   
   Ok finally I think we can use `roles` based on https://github.com/apache/hadoop-ozone/pull/1346#discussion_r481746270.
   
   I made necessary change in this PR. Can you take another look?


----------------------------------------------------------------
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 merged pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om roles`

Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375


   


----------------------------------------------------------------
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 a change in pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#discussion_r481521954



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/GetServiceRolesSubcommand.java
##########
@@ -30,10 +30,10 @@
 import java.util.concurrent.Callable;
 
 /**
- * Handler of om get-service-roles command.
+ * Handler of om status command.
  */
 @CommandLine.Command(
-    name = "getserviceroles",
+    name = "status",

Review comment:
       I think we should keep `getserviceroles` as an alias for compatibility.
   
   ```suggestion
       name = "status", aliases = "getserviceroles",
   ```




----------------------------------------------------------------
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] amaliujia commented on pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#issuecomment-685183677


   Context: https://github.com/apache/hadoop-ozone/pull/1346#discussion_r481380452


----------------------------------------------------------------
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] amaliujia commented on pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#issuecomment-685278548


   It seemed that there is another opinion about what command name should be. So I will convert this PR to a draft now to wait for a consensus. 
   
   Thanks for all reviews so far and sorry for the confusion.


----------------------------------------------------------------
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] amaliujia commented on pull request #1375: HDDS-4189. Change `ozone admin om getserviceroles` to `ozone admin om status`

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1375:
URL: https://github.com/apache/hadoop-ozone/pull/1375#issuecomment-685211998


   Thank you for your review @cxorm!
   
   ```
   [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 4.417 s <<< FAILURE! - in org.apache.hadoop.ozone.om.request.key.TestOMAllocateBlockRequest
   [ERROR] testValidateAndUpdateCache(org.apache.hadoop.ozone.om.request.key.TestOMAllocateBlockRequest)  Time elapsed: 0.114 s  <<< FAILURE!
   java.lang.AssertionError: Values should be different. Actual: 1599003025036
   	at org.junit.Assert.fail(Assert.java:88)
   	at org.junit.Assert.failEquals(Assert.java:185)
   	at org.junit.Assert.assertNotEquals(Assert.java:161)
   	at org.junit.Assert.assertNotEquals(Assert.java:198)
   	at org.junit.Assert.assertNotEquals(Assert.java:209)
   	at org.apache.hadoop.ozone.om.request.key.TestOMAllocateBlockRequest.testValidateAndUpdateCache(TestOMAllocateBlockRequest.java:100)
   	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.lang.reflect.Method.invoke(Method.java:498)
   	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
   	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
   
   ```
   
   The failed UT seems not related to this change.


----------------------------------------------------------------
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