You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/11/08 07:27:26 UTC

(pinot) branch master updated: Updated for commons-configuration2 in PinotConfiguartion (#11916)

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new e3f2777e36 Updated for commons-configuration2 in PinotConfiguartion (#11916)
e3f2777e36 is described below

commit e3f2777e365a094a84e47b4379e7e23410c42206
Author: Abhishek Sharma <ab...@spothero.com>
AuthorDate: Wed Nov 8 02:27:20 2023 -0500

    Updated for commons-configuration2 in PinotConfiguartion (#11916)
---
 .../apache/pinot/client/utils/ConnectionUtils.java |   2 +-
 .../org/apache/pinot/client/utils/DriverUtils.java |   2 +-
 .../apache/pinot/controller/ControllerConf.java    |   2 +-
 .../core/query/config/QueryExecutorConfig.java     |   2 +-
 .../pinot/core/query/executor/QueryExecutor.java   |   2 +-
 .../query/executor/ServerQueryExecutorV1Impl.java  |   2 +-
 .../executor/QueryExecutorExceptionsTest.java      |   8 +-
 .../core/query/executor/QueryExecutorTest.java     |   8 +-
 .../query/scheduler/PrioritySchedulerTest.java     |   2 +-
 .../pinot/queries/ExplainPlanQueriesTest.java      |   8 +-
 .../queries/SegmentWithNullValueVectorTest.java    |  10 +-
 .../org/apache/pinot/minion/MinionConfTest.java    |  15 +--
 .../pinot/spi/env/CommonsConfigurationUtils.java   | 118 ++++++++++++++++++---
 .../pinot/spi/env/ConfigFilePropertyReader.java    |   6 +-
 .../spi/env/ConfigFilePropertyReaderFactory.java   |   8 +-
 .../apache/pinot/spi/env/PinotConfiguration.java   |  30 +++---
 .../pinot/spi/env/PinotConfigurationTest.java      |  11 +-
 .../tools/filesystem/PinotFSBenchmarkDriver.java   |   9 +-
 18 files changed, 172 insertions(+), 73 deletions(-)

diff --git a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/utils/ConnectionUtils.java b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/utils/ConnectionUtils.java
index 51b8d57920..bc75140511 100644
--- a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/utils/ConnectionUtils.java
+++ b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/utils/ConnectionUtils.java
@@ -24,7 +24,7 @@ import java.util.Properties;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
-import org.apache.commons.configuration.MapConfiguration;
+import org.apache.commons.configuration2.MapConfiguration;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.pinot.common.config.TlsConfig;
diff --git a/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/utils/DriverUtils.java b/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/utils/DriverUtils.java
index c00b6faf1a..1bc7693c78 100644
--- a/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/utils/DriverUtils.java
+++ b/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/utils/DriverUtils.java
@@ -32,7 +32,7 @@ import java.util.Properties;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.net.ssl.SSLContext;
-import org.apache.commons.configuration.MapConfiguration;
+import org.apache.commons.configuration2.MapConfiguration;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.utils.URLEncodedUtils;
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
index b8e2247b08..1d7a5a5dac 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
@@ -26,7 +26,7 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
-import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration2.Configuration;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.helix.controller.rebalancer.strategy.AutoRebalanceStrategy;
 import org.apache.pinot.common.protocols.SegmentCompletionProtocol;
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/config/QueryExecutorConfig.java b/pinot-core/src/main/java/org/apache/pinot/core/query/config/QueryExecutorConfig.java
index ffe6341574..5c24536d9a 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/config/QueryExecutorConfig.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/config/QueryExecutorConfig.java
@@ -18,7 +18,7 @@
  */
 package org.apache.pinot.core.query.config;
 
-import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.utils.CommonConstants.Server;
 
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/QueryExecutor.java b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/QueryExecutor.java
index 3ec3fbe2e1..60727da79f 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/QueryExecutor.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/QueryExecutor.java
@@ -21,7 +21,7 @@ package org.apache.pinot.core.query.executor;
 import java.util.concurrent.ExecutorService;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
-import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.pinot.common.metrics.ServerMetrics;
 import org.apache.pinot.core.data.manager.InstanceDataManager;
 import org.apache.pinot.core.operator.blocks.InstanceResponseBlock;
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
index 17ce5ae159..ced6c62fb2 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
@@ -29,7 +29,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
-import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.lang.StringUtils;
 import org.apache.pinot.common.datatable.DataTable.MetadataKey;
 import org.apache.pinot.common.exception.QueryException;
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorExceptionsTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorExceptionsTest.java
index 82202007ab..65bb9a60a0 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorExceptionsTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorExceptionsTest.java
@@ -26,7 +26,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.helix.HelixManager;
@@ -53,6 +53,7 @@ import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.data.IngestionSchemaValidator;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.data.readers.FileFormat;
+import org.apache.pinot.spi.env.CommonsConfigurationUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.metrics.PinotMetricUtils;
 import org.apache.pinot.spi.utils.ReadMode;
@@ -154,9 +155,8 @@ public class QueryExecutorExceptionsTest {
     // Set up the query executor
     resourceUrl = getClass().getClassLoader().getResource(QUERY_EXECUTOR_CONFIG_PATH);
     assertNotNull(resourceUrl);
-    PropertiesConfiguration queryExecutorConfig = new PropertiesConfiguration();
-    queryExecutorConfig.setDelimiterParsingDisabled(false);
-    queryExecutorConfig.load(new File(resourceUrl.getFile()));
+    PropertiesConfiguration queryExecutorConfig =
+        CommonsConfigurationUtils.loadFromFile(new File(resourceUrl.getFile()));
     _queryExecutor = new ServerQueryExecutorV1Impl();
     _queryExecutor.init(new PinotConfiguration(queryExecutorConfig), instanceDataManager, _serverMetrics);
   }
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java
index f0cf7beecd..4398e796bf 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java
@@ -24,7 +24,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
 import org.apache.commons.io.FileUtils;
 import org.apache.helix.HelixManager;
 import org.apache.helix.store.zk.ZkHelixPropertyStore;
@@ -50,6 +50,7 @@ import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.data.IngestionSchemaValidator;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.data.readers.FileFormat;
+import org.apache.pinot.spi.env.CommonsConfigurationUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.metrics.PinotMetricUtils;
 import org.apache.pinot.spi.utils.ReadMode;
@@ -153,9 +154,8 @@ public class QueryExecutorTest {
     // Set up the query executor
     resourceUrl = getClass().getClassLoader().getResource(QUERY_EXECUTOR_CONFIG_PATH);
     Assert.assertNotNull(resourceUrl);
-    PropertiesConfiguration queryExecutorConfig = new PropertiesConfiguration();
-    queryExecutorConfig.setDelimiterParsingDisabled(false);
-    queryExecutorConfig.load(new File(resourceUrl.getFile()));
+    PropertiesConfiguration queryExecutorConfig =
+        CommonsConfigurationUtils.loadFromFile(new File(resourceUrl.getFile()));
     _queryExecutor = new ServerQueryExecutorV1Impl();
     _queryExecutor.init(new PinotConfiguration(queryExecutorConfig), instanceDataManager, _serverMetrics);
   }
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/scheduler/PrioritySchedulerTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/scheduler/PrioritySchedulerTest.java
index 66b5460c90..caeb453c55 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/query/scheduler/PrioritySchedulerTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/scheduler/PrioritySchedulerTest.java
@@ -36,7 +36,7 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.LongAccumulator;
 import javax.annotation.Nullable;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
 import org.apache.pinot.common.datatable.DataTable;
 import org.apache.pinot.common.datatable.DataTable.MetadataKey;
 import org.apache.pinot.common.datatable.DataTableFactory;
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java
index 4fd422b0f0..d68b4fb1ce 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java
@@ -30,7 +30,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
 import org.apache.commons.io.FileUtils;
 import org.apache.helix.HelixManager;
 import org.apache.helix.store.zk.ZkHelixPropertyStore;
@@ -67,6 +67,7 @@ import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.data.FieldSpec;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.env.CommonsConfigurationUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.metrics.PinotMetricUtils;
 import org.apache.pinot.spi.utils.CommonConstants;
@@ -314,9 +315,8 @@ public class ExplainPlanQueriesTest extends BaseQueriesTest {
     // Set up the query executor
     URL resourceUrl = getClass().getClassLoader().getResource(QUERY_EXECUTOR_CONFIG_PATH);
     Assert.assertNotNull(resourceUrl);
-    PropertiesConfiguration queryExecutorConfig = new PropertiesConfiguration();
-    queryExecutorConfig.setDelimiterParsingDisabled(false);
-    queryExecutorConfig.load(new File(resourceUrl.getFile()));
+    PropertiesConfiguration queryExecutorConfig =
+        CommonsConfigurationUtils.loadFromFile(new File(resourceUrl.getFile()));
     _queryExecutor = new ServerQueryExecutorV1Impl();
     _queryExecutor.init(new PinotConfiguration(queryExecutorConfig), instanceDataManager, _serverMetrics);
 
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/SegmentWithNullValueVectorTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/SegmentWithNullValueVectorTest.java
index 2de05b971e..1506a44426 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/SegmentWithNullValueVectorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/SegmentWithNullValueVectorTest.java
@@ -30,8 +30,8 @@ import java.util.Map;
 import java.util.Random;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.io.FileUtils;
 import org.apache.helix.HelixManager;
 import org.apache.helix.store.zk.ZkHelixPropertyStore;
@@ -61,6 +61,7 @@ import org.apache.pinot.spi.data.IngestionSchemaValidator;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.data.readers.GenericRow;
 import org.apache.pinot.spi.data.readers.RecordReader;
+import org.apache.pinot.spi.env.CommonsConfigurationUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.metrics.PinotMetricUtils;
 import org.apache.pinot.spi.utils.ReadMode;
@@ -169,9 +170,8 @@ public class SegmentWithNullValueVectorTest {
     // Set up the query executor
     URL resourceUrl = getClass().getClassLoader().getResource(QUERY_EXECUTOR_CONFIG_PATH);
     Assert.assertNotNull(resourceUrl);
-    PropertiesConfiguration queryExecutorConfig = new PropertiesConfiguration();
-    queryExecutorConfig.setDelimiterParsingDisabled(false);
-    queryExecutorConfig.load(new File(resourceUrl.getFile()));
+    PropertiesConfiguration queryExecutorConfig =
+        CommonsConfigurationUtils.loadFromFile(new File(resourceUrl.getFile()));
     _queryExecutor = new ServerQueryExecutorV1Impl();
     _queryExecutor.init(new PinotConfiguration(queryExecutorConfig), _instanceDataManager, _serverMetrics);
   }
diff --git a/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java b/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java
index 1f33840007..6ec33a5d59 100644
--- a/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java
+++ b/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java
@@ -18,8 +18,9 @@
  */
 package org.apache.pinot.minion;
 
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.pinot.spi.env.CommonsConfigurationUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.utils.CommonConstants;
 import org.testng.Assert;
@@ -38,9 +39,10 @@ public class MinionConfTest {
         CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER,
         CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_CRYPTER
     };
-    PinotConfiguration rawCfg = new PinotConfiguration(new PropertiesConfiguration(
+    PropertiesConfiguration config = CommonsConfigurationUtils.loadFromPath(
         PropertiesConfiguration.class.getClassLoader().getResource("pinot-configuration-old-minion.properties")
-            .getFile()));
+        .getFile());
+    PinotConfiguration rawCfg = new PinotConfiguration(config);
     final MinionConf oldConfig = new MinionConf(rawCfg.toMap());
     Assert.assertEquals(oldConfig.getMetricsPrefix(), "pinot.minion.old.custom.metrics.");
     for (String cfgKey : cfgKeys) {
@@ -49,9 +51,10 @@ public class MinionConfTest {
     }
 
     // Check configs with new names that have the pinot.minion prefix.
-    rawCfg = new PinotConfiguration(new PropertiesConfiguration(
+    config = CommonsConfigurationUtils.loadFromPath(
         PropertiesConfiguration.class.getClassLoader().getResource("pinot-configuration-new-minion.properties")
-            .getFile()));
+            .getFile());
+    rawCfg = new PinotConfiguration(config);
     final MinionConf newConfig = new MinionConf(rawCfg.toMap());
     for (String cfgKey : cfgKeys) {
       Assert.assertTrue(newConfig.subset(cfgKey).isEmpty(), cfgKey);
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java b/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
index 4feb4d33c2..fd9e84fd3a 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
@@ -32,9 +32,11 @@ import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.commons.configuration2.Configuration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.convert.DefaultListDelimiterHandler;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.io.FileHandler;
 import org.apache.commons.lang3.StringUtils;
 
 
@@ -42,17 +44,64 @@ import org.apache.commons.lang3.StringUtils;
  * Provide utility functions to manipulate Apache Commons {@link Configuration} instances.
  */
 public class CommonsConfigurationUtils {
+  private static final Character DEFAULT_LIST_DELIMITER = ',';
   private CommonsConfigurationUtils() {
   }
 
+  public static void setListDelimiterHandler(PropertiesConfiguration configuration, Character delimiter) {
+    configuration.setListDelimiterHandler(new DefaultListDelimiterHandler(delimiter));
+  }
+
+  public static void setDefaultListDelimiterHandler(PropertiesConfiguration configuration) {
+    setListDelimiterHandler(configuration, DEFAULT_LIST_DELIMITER);
+  }
+
+  public static PropertiesConfiguration loadFromPath(String path) throws ConfigurationException {
+    return loadFromPath(path, false, false);
+  }
+
+  public static PropertiesConfiguration loadFromPath(String path, boolean setIOFactory, boolean setDefaultDelimiter)
+      throws ConfigurationException {
+    PropertiesConfiguration config = createPropertiesConfiguration(setIOFactory, setDefaultDelimiter);
+    FileHandler fileHandler = new FileHandler(config);
+    fileHandler.load(path);
+    return config;
+  }
+
+  public static PropertiesConfiguration loadFromInputStream(InputStream stream) throws ConfigurationException {
+    return loadFromInputStream(stream, false, false);
+  }
+
+  public static PropertiesConfiguration loadFromInputStream(InputStream stream, boolean setIOFactory,
+      boolean setDefaultDelimiter) throws ConfigurationException {
+    PropertiesConfiguration config = createPropertiesConfiguration(setIOFactory, setDefaultDelimiter);
+    FileHandler fileHandler = new FileHandler(config);
+    fileHandler.load(stream);
+    return config;
+  }
+
+  public static PropertiesConfiguration loadFromFile(File file) throws ConfigurationException {
+    return loadFromFile(file, false, false);
+  }
+
+  public static PropertiesConfiguration loadFromFile(File file, boolean setIOFactory,
+      boolean setDefaultDelimiter) throws ConfigurationException {
+    PropertiesConfiguration config = createPropertiesConfiguration(setIOFactory, setDefaultDelimiter);
+    FileHandler fileHandler = new FileHandler(config);
+    fileHandler.load(file);
+    return config;
+  }
+
   /**
-   * Instantiate a {@link PropertiesConfiguration} from a {@link File}.
+   * Instantiate a {@link org.apache.commons.configuration.PropertiesConfiguration} from a {@link File}.
    * @param file containing properties
-   * @return a {@link PropertiesConfiguration} instance. Empty if file does not exist.
+   * @return a {@link org.apache.commons.configuration.PropertiesConfiguration} instance. Empty if file does not exist.
    */
-  public static PropertiesConfiguration fromFile(File file) {
+  @Deprecated
+  public static org.apache.commons.configuration.PropertiesConfiguration fromFile(File file) {
     try {
-      PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration();
+      org.apache.commons.configuration.PropertiesConfiguration propertiesConfiguration =
+          new org.apache.commons.configuration.PropertiesConfiguration();
 
       // Commons Configuration 1.10 does not support file path containing '%'.
       // Explicitly providing the input stream on load bypasses the problem.
@@ -62,32 +111,36 @@ public class CommonsConfigurationUtils {
       }
 
       return propertiesConfiguration;
-    } catch (ConfigurationException | FileNotFoundException e) {
+    } catch (org.apache.commons.configuration.ConfigurationException | FileNotFoundException e) {
       throw new RuntimeException(e);
     }
   }
 
   /**
-   * Instantiate a {@link PropertiesConfiguration} from an inputstream.
+   * Instantiate a {@link org.apache.commons.configuration.PropertiesConfiguration} from an inputstream.
    * @param inputStream containing properties
-   * @return a {@link PropertiesConfiguration} instance.
+   * @return a {@link org.apache.commons.configuration.PropertiesConfiguration} instance.
    */
-  public static PropertiesConfiguration fromInputStream(InputStream inputStream) {
+  @Deprecated
+  public static org.apache.commons.configuration.PropertiesConfiguration fromInputStream(InputStream inputStream) {
     try {
-      PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration();
+      org.apache.commons.configuration.PropertiesConfiguration propertiesConfiguration =
+          new org.apache.commons.configuration.PropertiesConfiguration();
       propertiesConfiguration.load(inputStream);
       return propertiesConfiguration;
-    } catch (ConfigurationException e) {
+    } catch (org.apache.commons.configuration.ConfigurationException e) {
       throw new RuntimeException(e);
     }
   }
 
-  public static void saveToFile(PropertiesConfiguration propertiesConfiguration, File file) {
+  @Deprecated
+  public static void saveToFile(org.apache.commons.configuration.PropertiesConfiguration propertiesConfiguration,
+      File file) {
     // Commons Configuration 1.10 does not support file path containing '%'.
     // Explicitly providing the output stream for save bypasses the problem.
     try (FileOutputStream fileOutputStream = new FileOutputStream(file)) {
       propertiesConfiguration.save(fileOutputStream);
-    } catch (ConfigurationException | IOException e) {
+    } catch (org.apache.commons.configuration.ConfigurationException | IOException e) {
       throw new RuntimeException(e);
     }
   }
@@ -116,10 +169,15 @@ public class CommonsConfigurationUtils {
    * @param configuration to iterate on keys
    * @return a list of keys
    */
+  @Deprecated
   public static List<String> getKeys(org.apache.commons.configuration.Configuration configuration) {
     return getKeysStream(configuration).collect(Collectors.toList());
   }
 
+  public static List<String> getKeys(Configuration configuration) {
+    return getKeysStream(configuration).collect(Collectors.toList());
+  }
+
   /**
    * @return a key-value {@link Map} found in the provided {@link Configuration}
    */
@@ -189,6 +247,22 @@ public class CommonsConfigurationUtils {
     }
   }
 
+  @SuppressWarnings("unchecked")
+  public static <T> T interpolate(Configuration configuration, String key, T defaultValue, Class<T> returnType) {
+    // Different from the generic getProperty() method, those type specific getters do config interpolation.
+    if (Integer.class.equals(returnType)) {
+      return (T) configuration.getInteger(key, (Integer) defaultValue);
+    } else if (Boolean.class.equals(returnType)) {
+      return (T) configuration.getBoolean(key, (Boolean) defaultValue);
+    } else if (Long.class.equals(returnType)) {
+      return (T) configuration.getLong(key, (Long) defaultValue);
+    } else if (Double.class.equals(returnType)) {
+      return (T) configuration.getDouble(key, (Double) defaultValue);
+    } else {
+      throw new IllegalArgumentException(returnType + " is not a supported type for conversion.");
+    }
+  }
+
   @SuppressWarnings("unchecked")
   public static <T> T convert(Object value, Class<T> returnType) {
     if (Integer.class.equals(returnType)) {
@@ -242,4 +316,20 @@ public class CommonsConfigurationUtils {
     }
     return value.replace("\0\0", ",");
   }
+
+  private static PropertiesConfiguration createPropertiesConfiguration(boolean setIOFactory,
+      boolean setDefaultDelimiter) {
+    PropertiesConfiguration config = new PropertiesConfiguration();
+
+    // setting IO Reader Factory
+    if (setIOFactory) {
+      config.setIOFactory(new ConfigFilePropertyReaderFactory());
+    }
+
+    // setting DEFAULT_LIST_DELIMITER
+    if (setDefaultDelimiter) {
+      CommonsConfigurationUtils.setDefaultListDelimiterHandler(config);
+    }
+    return config;
+  }
 }
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReader.java b/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReader.java
index ed9bf46f4e..affbad8414 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReader.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReader.java
@@ -19,13 +19,13 @@
 package org.apache.pinot.spi.env;
 
 import java.io.Reader;
-import org.apache.commons.configuration.PropertiesConfiguration.PropertiesReader;
+import org.apache.commons.configuration2.PropertiesConfiguration.PropertiesReader;
 
 
 public class ConfigFilePropertyReader extends PropertiesReader {
 
-  public ConfigFilePropertyReader(Reader reader, char listDelimiter) {
-    super(reader, listDelimiter);
+  public ConfigFilePropertyReader(Reader reader) {
+    super(reader);
   }
 
   @Override
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReaderFactory.java b/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReaderFactory.java
index e6748c63fc..6de66bad7c 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReaderFactory.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/ConfigFilePropertyReaderFactory.java
@@ -19,13 +19,13 @@
 package org.apache.pinot.spi.env;
 
 import java.io.Reader;
-import org.apache.commons.configuration.PropertiesConfiguration.DefaultIOFactory;
-import org.apache.commons.configuration.PropertiesConfiguration.PropertiesReader;
+import org.apache.commons.configuration2.PropertiesConfiguration.DefaultIOFactory;
+import org.apache.commons.configuration2.PropertiesConfiguration.PropertiesReader;
 
 
 public class ConfigFilePropertyReaderFactory extends DefaultIOFactory {
   @Override
-  public PropertiesReader createPropertiesReader(Reader in, char delimiter) {
-    return new ConfigFilePropertyReader(in, delimiter);
+  public PropertiesReader createPropertiesReader(Reader in) {
+    return new ConfigFilePropertyReader(in);
   }
 }
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
index 7bd23f503c..49e8b381e8 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
@@ -26,11 +26,12 @@ import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import org.apache.commons.configuration.CompositeConfiguration;
-import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.MapConfiguration;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.CompositeConfiguration;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.commons.configuration2.MapConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.convert.DefaultListDelimiterHandler;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.pinot.spi.ingestion.batch.spec.PinotFSSpec;
 import org.apache.pinot.spi.utils.Obfuscator;
 
@@ -162,8 +163,11 @@ public class PinotConfiguration {
 
         .map(PinotConfiguration::loadProperties);
 
-    return Stream.concat(Stream.of(relaxedBaseProperties, relaxedEnvVariables).map(MapConfiguration::new),
-        propertiesFromConfigPaths).collect(Collectors.toList());
+    return Stream.concat(Stream.of(relaxedBaseProperties, relaxedEnvVariables).map(e -> {
+      MapConfiguration mapConfiguration = new MapConfiguration(e);
+      mapConfiguration.setListDelimiterHandler(new DefaultListDelimiterHandler(','));
+      return mapConfiguration;
+    }), propertiesFromConfigPaths).collect(Collectors.toList());
   }
 
   private static String getProperty(String name, Configuration configuration) {
@@ -180,16 +184,14 @@ public class PinotConfiguration {
 
   private static Configuration loadProperties(String configPath) {
     try {
-      PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration();
-
-      propertiesConfiguration.setIOFactory(new ConfigFilePropertyReaderFactory());
+      PropertiesConfiguration propertiesConfiguration;
       if (configPath.startsWith("classpath:")) {
-        propertiesConfiguration
-            .load(PinotConfiguration.class.getResourceAsStream(configPath.substring("classpath:".length())));
+        propertiesConfiguration = CommonsConfigurationUtils.loadFromInputStream(
+            PinotConfiguration.class.getResourceAsStream(configPath.substring("classpath:".length())),
+            true, true);
       } else {
-        propertiesConfiguration.load(configPath);
+        propertiesConfiguration = CommonsConfigurationUtils.loadFromPath(configPath, true, true);
       }
-
       return propertiesConfiguration;
     } catch (ConfigurationException e) {
       throw new IllegalArgumentException("Could not read properties from " + configPath, e);
diff --git a/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java b/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
index 0b33d73d71..eb686e9e67 100644
--- a/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
@@ -28,8 +28,8 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.pinot.spi.ingestion.batch.spec.PinotFSSpec;
 import org.testng.Assert;
 import org.testng.annotations.Test;
@@ -179,8 +179,11 @@ public class PinotConfigurationTest {
   @Test
   public void assertPropertiesFromBaseConfiguration()
       throws ConfigurationException {
-    PinotConfiguration config = new PinotConfiguration(new PropertiesConfiguration(
-        PropertiesConfiguration.class.getClassLoader().getResource("pinot-configuration-1.properties").getFile()));
+    PropertiesConfiguration propertiesConfiguration = CommonsConfigurationUtils.loadFromPath(
+        PropertiesConfiguration.class.getClassLoader().getResource("pinot-configuration-1.properties").getFile(),
+        true, true);
+
+    PinotConfiguration config = new PinotConfiguration(propertiesConfiguration);
 
     Assert.assertEquals(config.getProperty("pinot.server.storage.factory.class.s3"),
         "org.apache.pinot.plugin.filesystem.S3PinotFS");
diff --git a/pinot-tools/src/main/java/org/apache/pinot/tools/filesystem/PinotFSBenchmarkDriver.java b/pinot-tools/src/main/java/org/apache/pinot/tools/filesystem/PinotFSBenchmarkDriver.java
index c9a234d645..14f998a798 100644
--- a/pinot-tools/src/main/java/org/apache/pinot/tools/filesystem/PinotFSBenchmarkDriver.java
+++ b/pinot-tools/src/main/java/org/apache/pinot/tools/filesystem/PinotFSBenchmarkDriver.java
@@ -24,10 +24,10 @@ import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.net.URI;
 import java.net.URISyntaxException;
-import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.io.FileUtils;
+import org.apache.pinot.spi.env.CommonsConfigurationUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.filesystem.PinotFS;
 import org.apache.pinot.spi.filesystem.PinotFSFactory;
@@ -53,7 +53,8 @@ public class PinotFSBenchmarkDriver {
   public PinotFSBenchmarkDriver(String mode, String configFilePath, String baseDirectoryUri, String localTempDir,
       Integer numSegmentsForListFilesTest, Integer dataSizeInMBsForCopyTest, Integer numOps)
       throws ConfigurationException {
-    Configuration configuration = new PropertiesConfiguration(new File(configFilePath));
+    PropertiesConfiguration configuration =
+        CommonsConfigurationUtils.loadFromFile(new File(configFilePath));
     PinotFSFactory.init(new PinotConfiguration(configuration));
     _mode = mode;
     _baseDirectoryUri = URI.create(baseDirectoryUri);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org