[GitHub] [hadoop-ozone] elek commented on a change in pull request #1106: HDDS-2413. Set configuration variables from annotated java objects

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

elek commented on a change in pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106#discussion_r447520786



##########
File path: hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
##########
@@ -37,9 +39,6 @@
 
   char[] getPassword(String key) throws IOException;
 
-  @Deprecated

Review comment:
       I really don't like the approach when the current port number (after using :0) is added to the configuration. It's easier if the configuration is immutable after loading.
   
   I understand that the immutable is not possible with this injection and I accept that it's necessary.
   
   One possible approach is to introduce two interfaces: `ConfigSource` (read-only) and `ConfigDestination` (write only). `OzoneConfiguration` can implement both. But there could be a specific `ConfigSource` (eg. `EnvironmentVariableConfigSource`) which would be read only. 
   
   But I am fine with committing this patch as is, but interested about opinion.
   
   Also: it seems to be useful for testing to introduce an `OzoneConfiguration.fromAnnotatedObject()` method.
    
   
   




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