You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "abhioncbr (via GitHub)" <gi...@apache.org> on 2023/11/01 10:25:52 UTC
[PR] Updated for commons-configuration2 in PinotConfiguartion [pinot]
abhioncbr opened a new pull request, #11916:
URL: https://github.com/apache/pinot/pull/11916
In continuation of the previous work https://github.com/apache/pinot/pull/11792 & https://github.com/apache/pinot/pull/11868, This PR upgrade the `PinotConfiguartion` to `commons-configuartion2`
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Updated for commons-configuration2 in PinotConfiguartion [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #11916:
URL: https://github.com/apache/pinot/pull/11916#issuecomment-1799538701
@Jackie-Jiang, Can you please review it once more? Thanks
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Updated for commons-configuration2 in PinotConfiguartion [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Updated for commons-configuration2 in PinotConfiguartion [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11916:
URL: https://github.com/apache/pinot/pull/11916#issuecomment-1788796124
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11916](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (36bba9f) into [master](https://app.codecov.io/gh/apache/pinot/commit/abad6c8d26f7f3d2d43b6fc6dd5692a7952ce77a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (abad6c8) will **decrease** coverage by `61.50%`.
> Report is 4 commits behind head on master.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11916 +/- ##
=============================================
- Coverage 61.50% 0.00% -61.50%
+ Complexity 1147 6 -1141
=============================================
Files 2378 2302 -76
Lines 128844 125126 -3718
Branches 19925 19371 -554
=============================================
- Hits 79242 6 -79236
- Misses 43858 125120 +81262
+ Partials 5744 0 -5744
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.45%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.48%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.50%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11916/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...org/apache/pinot/client/utils/ConnectionUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L3V0aWxzL0Nvbm5lY3Rpb25VdGlscy5qYXZh) | `0.00% <ø> (-50.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/client/utils/DriverUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qZGJjLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L3V0aWxzL0RyaXZlclV0aWxzLmphdmE=) | `0.00% <ø> (-19.10%)` | :arrow_down: |
| [...va/org/apache/pinot/controller/ControllerConf.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyQ29uZi5qYXZh) | `0.00% <ø> (-60.83%)` | :arrow_down: |
| [...e/pinot/core/query/config/QueryExecutorConfig.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9jb25maWcvUXVlcnlFeGVjdXRvckNvbmZpZy5qYXZh) | `0.00% <ø> (-85.72%)` | :arrow_down: |
| [...pache/pinot/core/query/executor/QueryExecutor.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9RdWVyeUV4ZWN1dG9yLmphdmE=) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...core/query/executor/ServerQueryExecutorV1Impl.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9TZXJ2ZXJRdWVyeUV4ZWN1dG9yVjFJbXBsLmphdmE=) | `0.00% <ø> (-64.60%)` | :arrow_down: |
| [...apache/pinot/spi/env/ConfigFilePropertyReader.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L0NvbmZpZ0ZpbGVQcm9wZXJ0eVJlYWRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/spi/env/ConfigFilePropertyReaderFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L0NvbmZpZ0ZpbGVQcm9wZXJ0eVJlYWRlckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/env/PinotConfiguration.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L1Bpbm90Q29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (-94.19%)` | :arrow_down: |
| [...pache/pinot/spi/env/CommonsConfigurationUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11916?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L0NvbW1vbnNDb25maWd1cmF0aW9uVXRpbHMuamF2YQ==) | `0.00% <0.00%> (-70.84%)` | :arrow_down: |
... and [1976 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11916/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Updated for commons-configuration2 in PinotConfiguartion [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11916:
URL: https://github.com/apache/pinot/pull/11916
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org