[GitHub] [zeppelin] Reamer commented on a change in pull request #4122: [ZEPPELIN-5379] LIST_CONFIGURATIONS command cannot return the default…

[GitBox]

Reamer commented on a change in pull request #4122:
URL: https://github.com/apache/zeppelin/pull/4122#discussion_r635821821



##########
File path: 
zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
##########
@@ -871,6 +871,10 @@ public boolean isPrometheusMetricEnabled() {
         completeConfiguration.put(c.getVarName(), 
sysConfig.getString(c.getVarName()));
       } else if (envConfig.containsKey(c.name())) {
         completeConfiguration.put(c.getVarName(), 
envConfig.getString(c.name()));
+      }else {
+        if (getString(c)!=null){
+          completeConfiguration.put(c.getVarName(),getString(c));
+        }

Review comment:
       > Since `envConfig` and `sysConfig` are checked in `getString()`, would 
it be okay to delete lines 870~873?
   
   You should be right. I think we can also start with an empty Hashmap.
   ```java
     public Map<String, String> getCompleteConfiguration() {
       Map<String, String> completeConfiguration = new HashMap<>();
       for (ConfVars c : ConfVars.values()) {
           if (getString(c) != null){
               completeConfiguration.put(c.getVarName(), getString(c));
           }
       }
       return completeConfiguration;
     }
   ```




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