Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#841] feat(config): Support deprecated and fallback keys for ConfigOptions #843

Merged
merged 3 commits into from
Apr 27, 2023

Conversation

zuston
Copy link
Member

@zuston zuston commented Apr 26, 2023

What changes were proposed in this pull request?

Support deprecated and fallback keys for ConfigOptions

Why are the changes needed?

#841 . To achieve better compatibility, it's time to support deprecated keys for ConfigOptions .

After this PR, the configOption will have the fallback and deprecated keys. Like this

    final ConfigOption<Integer> intConfig = ConfigOptions
        .key("rss.main.key")
        .intType()
        .defaultValue(100)
        .withFallbackKeys("rss.fallback.key1")
        .withDeprecatedKeys("rss.deprecated.key1");

Does this PR introduce any user-facing change?

No.

How was this patch tested?

  1. Existing UTs
  2. Newly UTs

@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2023

Codecov Report

Merging #843 (3161b80) into master (c58110f) will increase coverage by 2.89%.
The diff coverage is 91.42%.

@@             Coverage Diff              @@
##             master     #843      +/-   ##
============================================
+ Coverage     57.13%   60.03%   +2.89%     
+ Complexity     2147     2030     -117     
============================================
  Files           321      289      -32     
  Lines         15655    12050    -3605     
  Branches       1243     1125     -118     
============================================
- Hits           8945     7234    -1711     
+ Misses         6204     4410    -1794     
+ Partials        506      406     -100     
Impacted Files Coverage Δ
.../org/apache/uniffle/common/config/FallbackKey.java 77.77% <77.77%> (ø)
...org/apache/uniffle/common/config/ConfigOption.java 74.35% <94.11%> (+13.48%) ⬆️
...java/org/apache/uniffle/common/config/RssConf.java 33.73% <100.00%> (+3.79%) ⬆️

... and 36 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zuston zuston changed the title [#841] feat(config): Support deprecated keys for ConfigOptions [#841] feat(config): Support deprecated and fallback keys for ConfigOptions Apr 26, 2023
@zuston
Copy link
Member Author

zuston commented Apr 26, 2023

The code of configOption in uniffle refers to Flink implementation. So in this PR, I still use the Flink's part of code to achieve this.

@jerqi
Copy link
Contributor

jerqi commented Apr 26, 2023

LGTM, let @smallzhongfeng take another look.

Copy link
Contributor

@smallzhongfeng smallzhongfeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@zuston zuston merged commit 24a5195 into apache:master Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants