[#796][0.7] bug: Fix the issues of MetricReporter by xianjingfeng · Pull Request #821 · apache/uniffle · GitHub
Skip to content

[#796][0.7] bug: Fix the issues of MetricReporter#821

Merged
xianjingfeng merged 4 commits into
apache:branch-0.7from
xianjingfeng:796_for_0.7
Apr 13, 2023
Merged

[#796][0.7] bug: Fix the issues of MetricReporter#821
xianjingfeng merged 4 commits into
apache:branch-0.7from
xianjingfeng:796_for_0.7

Conversation

@xianjingfeng

@xianjingfeng xianjingfeng commented Apr 13, 2023

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

  1. Support custom config keys defined in plugins
  2. Refactor the logic for load config file
  3. Fix some issues of metricReporter.

Why are the changes needed?

Metric reporter is unusable.
Fix: #796

Does this PR introduce any user-facing change?

No.

How was this patch tested?

UT and Manual testing

@xianjingfeng
[apache#796] bug:fix the issues of MetricReporter (apache#797)
0507c87 
### What changes were proposed in this pull request?
Support custom config keys defined in plugins
Refactor the logic for load config file
Fix some issues of metricReporter.
### Why are the changes needed?
Metric reporter is unusable.
Fix: apache#796

### Does this PR introduce any user-facing change?
No.

### How was this patch tested?
UT and Manual testing
# Conflicts:
#	common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java
#	server/src/test/java/org/apache/uniffle/server/ShuffleServerConfTest.java
@xianjingfeng xianjingfeng requested review from roryqi and zuston April 13, 2023 02:07
@xianjingfeng xianjingfeng mentioned this pull request Apr 13, 2023
Merged
@roryqi roryqi changed the title [#796] bug:fix the issues of MetricReporter(for 0.7) [#796][0.7] bug:fix the issues of MetricReporter Apr 13, 2023
@codecov-commenter

codecov-commenter commented Apr 13, 2023 •
edited
Loading

Copy link
Copy Markdown

@xianjingfeng

xianjingfeng commented Apr 13, 2023

Copy link
Copy Markdown
Member Author

It seems that this PR is merged to master in CI enviroment. But the first time is ok.
https://github.com/apache/incubator-uniffle/actions/runs/4684746312/jobs/8301194063

Do you know why? @kaijchen @jerqi

@roryqi

roryqi commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

It seems that this PR is merged to master in CI enviroment. But the first time is ok. https://github.com/apache/incubator-uniffle/actions/runs/4684746312/jobs/8301194063

Do you know why? @kaijchen @jerqi

I cherry-pick a wrong commit. I have reverted it. it's strange that it influence your pr. You could rerun your pr.

@kaijchen

kaijchen commented Apr 13, 2023

Copy link
Copy Markdown
Member

You could rerun your pr.

Please rebase or merge branch-0.7.

@roryqi

roryqi commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

You could rerun your pr.

Please rebase or merge branch-0.7.

企业微信截图_90010f5d-d8b3-4afd-9ad7-45b8ba63e889

But his commit history don't have my mistake commit. It's strange. But the error seems that it caused by my mistake commit.

@xianjingfeng xianjingfeng reopened this Apr 13, 2023
@xianjingfeng

xianjingfeng commented Apr 13, 2023

Copy link
Copy Markdown
Member Author

roryqi
roryqi approved these changes Apr 13, 2023
View reviewed changes

@roryqi roryqi 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, thanks @xianjingfeng

@roryqi roryqi changed the title [#796][0.7] bug:fix the issues of MetricReporter [#796][0.7] bug: Fix the issues of MetricReporter Apr 13, 2023
@xianjingfeng xianjingfeng merged commit bcb591e into apache:branch-0.7 Apr 13, 2023
@xianjingfeng xianjingfeng deleted the 796_for_0.7 branch April 13, 2023 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@roryqi roryqi roryqi approved these changes
@zuston zuston Awaiting requested review from zuston

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xianjingfeng @codecov-commenter @roryqi @kaijchen