Jackie-Jiang commented on code in PR #11916:
URL: https://github.com/apache/pinot/pull/11916#discussion_r1382293393
##########
pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java:
##########
@@ -32,27 +32,54 @@
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;
/**
* 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 setDefaultListDelimiterHandler(PropertiesConfiguration
configuration) {
+ configuration.setListDelimiterHandler(new
DefaultListDelimiterHandler(DEFAULT_LIST_DELIMITER));
+ }
+
+ public static void loadPropertiesConfiguration(PropertiesConfiguration
configuration, String path)
Review Comment:
Same for other loading methods, we can directly return
`PropertiesConfiguration`
```suggestion
public static PropertiesConfiguration loadFromFile(String filePath)
```
##########
pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorExceptionsTest.java:
##########
@@ -155,8 +156,8 @@ public void setUp()
resourceUrl =
getClass().getClassLoader().getResource(QUERY_EXECUTOR_CONFIG_PATH);
assertNotNull(resourceUrl);
PropertiesConfiguration queryExecutorConfig = new
PropertiesConfiguration();
- queryExecutorConfig.setDelimiterParsingDisabled(false);
- queryExecutorConfig.load(new File(resourceUrl.getFile()));
+ FileHandler fileHandler = new FileHandler(queryExecutorConfig);
Review Comment:
Use `CommonsConfigurationUtils.loadFromFile(resourceUrl.getFile())`, same
for other places where config needs to be loaded
--
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]
