tomicooler commented on code in PR #4655:
URL: https://github.com/apache/hadoop/pull/4655#discussion_r991274835


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java:
##########
@@ -1200,7 +1207,24 @@ public ConfigurationProperties 
getConfigurationProperties() {
   public void reinitializeConfigurationProperties() {
     // Props are always Strings, therefore this cast is safe
     Map<String, String> props = (Map) getProps();
-    configurationProperties = new ConfigurationProperties(props);
+    configurationProperties = new ConfigurationProperties(props, PREFIX);
+  }
+
+  @Override
+  public void set(String name, String value) {
+    super.set(name, value);
+    if (configurationProperties != null) {
+      //The super#get method used cause the super#set contains some logic
+      configurationProperties.set(super.get(name), value);

Review Comment:
   I don't think that this is enough. The set may create multiple entries in 
the configuration (deprecation handling logic and alternative names), the get() 
just returns one (last?). (The documentation lies, it says "first"):
   
   ```
     /** 
      * Get the value of the <code>name</code>. If the key is deprecated,
      * it returns the value of the first key which replaces the deprecated key
      * and is not null.
      * If no such property exists,
      * then <code>defaultValue</code> is returned.
      * 
      * @param name property name, will be trimmed before get value.
      * @param defaultValue default value.
      * @return property value, or <code>defaultValue</code> if the property 
      *         doesn't exist.                    
      */
     public String get(String name, String defaultValue) {
       String[] names = handleDeprecation(deprecationContext.get(), name);
       String result = null;
       for(String n : names) {
         result = substituteVars(getProps().getProperty(n, defaultValue));
       }
       return result;
     }
   ```
   
   The Configuration class is marked as stable API, so it's hard to change 
these methods. I'm not sure if there is any way to know which configs were set 
during this call other than copying the object then comparing it the the 
altered.
   
   Need to investigate if this is even feasible (keeping everything in sync).



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