[GitHub] [hadoop] elek commented on a change in pull request #713: HDDS-1192. Support -conf command line argument in GenericCli

[GitBox <gi...@apache.org>]

elek commented on a change in pull request #713: HDDS-1192. Support -conf command line argument in GenericCli
URL: https://github.com/apache/hadoop/pull/713#discussion_r276196228
 
 

 ##########
 File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java
 ##########
 @@ -161,6 +157,8 @@ public void start(Object service) {
     OzoneConfiguration.activate();
     if (service instanceof Configurable) {
       conf = new OzoneConfiguration(((Configurable) service).getConf());
+    } else {
 
 Review comment:
   I think this part can be simplified (but fix me if I am wrong) with using two start method:
   
   ```
    /**
      * Starts HddsDatanode services.
      *
      * @param service The service instance invoking this method
      */
     @Override
     public void start(Object service) {
       if (service instanceof Configurable) {
         start(new OzoneConfiguration(((Configurable) service).getConf()));
       } else {
         start(new OzoneConfiguration());
       }
     }
   
     public void start(OzoneConfiguration conf) {
       this.conf = conf;
       DefaultMetricsSystem.initialize("HddsDatanode");
       OzoneConfiguration.activate();
       if (HddsUtils.isHddsEnabled(conf)) {
    ```
   
   In this case we don't need to add the additional method to the GenericParentCommand so it can be as simple as now.
   
   And from the new call() method we can use the createOzoneConfiguration()
   
   ```
     @Override
     public Void call() throws Exception {
       if (printBanner) {
         StringUtils
             .startupShutdownMessage(HddsDatanodeService.class, args, LOG);
       }
       start(createOzoneConfiguration());
       join();
       return null;
     }
   ```
   
   initConf also can be removed with this approach (IMHO)
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org