Skip to content

[AMORO-4211] Fix legacy Flink config loading#4249

Merged
czy006 merged 1 commit into
apache:masterfrom
NikhilUjjwal7:fix-4211-flink-config-yaml
Jun 24, 2026
Merged

[AMORO-4211] Fix legacy Flink config loading#4249
czy006 merged 1 commit into
apache:masterfrom
NikhilUjjwal7:fix-4211-flink-config-yaml

Conversation

@NikhilUjjwal7

@NikhilUjjwal7 NikhilUjjwal7 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Why are the changes needed?

Close #4211.

Legacy flink-conf.yaml can contain Flink metric filter values that start with *, for example:

xxx.metrics.filter.includes: *RSS:*:*;Heap:Used,Max:*

Close #xxx.

## Brief change log
Updated FlinkOptimizerContainer to load legacy flink-conf.yaml using Flink GlobalConfiguration.
Kept existing config.yaml loading behavior unchanged.
Added a unit-level regression test for legacy flink-conf.yaml containing metric filter values starting with *.

## How was this patch tested?

Added regression test :

TestFlinkOptimizerContainer#testReadLegacyFlinkConfigWithMetricFilterIncludes

Add screenshots for manual tests if appropriate
<img width="970" height="691" alt="Screenshot 2026-06-11 at 10 33 50 AM" src="https://github.com/user-attachments/assets/e5c7dad3-c761-41bc-9f2f-a69ca195ff8c" />



Run test locally before making a pull request
Ran locally :

./mvnw -pl amoro-ams -am \
  -Dtest=TestFlinkOptimizerContainer#testReadLegacyFlinkConfigWithMetricFilterIncludes \
  -Dsurefire.failIfNoSpecifiedTests=false \
  test
  
 Result : 
 Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
  
  
## Documentation


Does this pull request introduce a new feature? no
If yes, how is the feature documented? not applicable

@github-actions github-actions Bot added the module:ams-server Ams server module label Jun 11, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.06%. Comparing base (99fcc08) to head (dd27ec4).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4249      +/-   ##
============================================
+ Coverage     23.09%   30.06%   +6.96%     
- Complexity     2706     4380    +1674     
============================================
  Files           463      680     +217     
  Lines         42826    55261   +12435     
  Branches       6044     7079    +1035     
============================================
+ Hits           9891    16614    +6723     
- Misses        32076    37395    +5319     
- Partials        859     1252     +393     
Flag Coverage Δ
core 30.06% <100.00%> (?)
trino ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@czy006 czy006 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@czy006 czy006 merged commit 2b3027d into apache:master Jun 24, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Failed to load flink config

4 participants