[GitHub] keith-turner commented on a change in pull request #357: ACCUMULO-4611 Deprecate commons config in api

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

keith-turner commented on a change in pull request #357: ACCUMULO-4611 Deprecate commons config in api
URL: https://github.com/apache/accumulo/pull/357#discussion_r161800073
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java
 ##########
 @@ -147,30 +154,46 @@ public static ClientProperty getPropertyByKey(String key) {
     }
   }
 
-  public ClientConfiguration(String configFile) throws ConfigurationException {
-    this(new PropertiesConfiguration(), configFile);
+  // helper for the constructor which takes a String file name
+  private static PropertiesConfiguration newPropsFile(String file) throws ConfigurationException {
+    PropertiesConfiguration props = new PropertiesConfiguration();
+    props.setListDelimiter('\0');
+    props.load(file);
+    return props;
   }
 
-  private ClientConfiguration(PropertiesConfiguration propertiesConfiguration, String configFile) throws ConfigurationException {
-    super(propertiesConfiguration);
-    // Don't do list interpolation
-    this.setListDelimiter('\0');
-    propertiesConfiguration.setListDelimiter('\0');
-    propertiesConfiguration.load(configFile);
+  // helper for the constructor which takes a File
+  private static PropertiesConfiguration newPropsFile(File file) throws ConfigurationException {
+    PropertiesConfiguration props = new PropertiesConfiguration();
+    props.setListDelimiter('\0');
+    props.load(file);
+    return props;
   }
 
-  public ClientConfiguration(File configFile) throws ConfigurationException {
-    this(new PropertiesConfiguration(), configFile);
+  /**
+   * @deprecated since 1.9.0; removing commons-configuration leakage into the API in 2.0.0; use {@link #fromFile(File)} instead.
+   */
+  @Deprecated
+  public ClientConfiguration(String configFile) throws ConfigurationException {
+    this(Collections.singletonList(newPropsFile(configFile)));
   }
 
-  private ClientConfiguration(PropertiesConfiguration propertiesConfiguration, File configFile) throws ConfigurationException {
-    super(propertiesConfiguration);
-    // Don't do list interpolation
-    this.setListDelimiter('\0');
-    propertiesConfiguration.setListDelimiter('\0');
-    propertiesConfiguration.load(configFile);
+  /**
+   * Load a client configuration from the provided configuration properties file
+   *
+   * @param configFile
+   *          the path to the properties file
+   * @deprecated since 1.9.0; removing commons-configuration leakage into the API in 2.0.0; use {@link #fromFile(File)} instead.
+   */
+  @Deprecated
+  public ClientConfiguration(File configFile) throws ConfigurationException {
+    this(Collections.singletonList(newPropsFile(configFile)));
   }
 
+  /**
+   * @deprecated since 1.9.0; removing commons-configuration leakage into the API in 2.0.0
 
 Review comment:
   Personally I think a phrasing like `will be removed in 2.0.0 to eliminate commons config leakage into Accumulo API` is a bit more clear, but I Am ok with current phrasing.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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