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.

Sign up for GitHub

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

Jump to bottom

[Optimization] Config module resource relation fix. #10718

Merged
merged 8 commits into from
Jul 5, 2022
Merged

[Optimization] Config module resource relation fix. #10718

merged 8 commits into from
Jul 5, 2022

Conversation

WangJPLeo
Copy link
Contributor

@WangJPLeo WangJPLeo commented Jun 30, 2022 •
edited
Loading

Purpose of the pull request

  1. Config module resource relation fix.
  2. Add transaction handling on create data resource method that requires transaction.

Brief change log

close #10717

Verify this pull request

Added CronUtilsTest to verify the change.
Manually verified the change by testing locally.

@WangJPLeo WangJPLeo changed the title [] Config module resource relation fix. [Optimization] Config module resource relation fix. Jun 30, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2022 •
edited
Loading

Codecov Report

Merging #10718 (b7f34f3) into dev (7898f41) will decrease coverage by 0.22%.
The diff coverage is 28.41%.

❗ Current head b7f34f3 differs from pull request most recent head 872e4f9. Consider uploading reports for the commit 872e4f9 to get more accurate results

@@             Coverage Diff              @@
##                dev   #10718      +/-   ##
============================================
- Coverage     40.84%   40.61%   -0.23%     
+ Complexity     4826     4817       -9     
============================================
  Files           897      902       +5     
  Lines         36127    36221      +94     
  Branches       3987     3999      +12     
============================================
- Hits          14755    14711      -44     
- Misses        19907    20045     +138     
  Partials       1465     1465
Impacted Files Coverage Δ
...eduler/api/service/impl/AlertGroupServiceImpl.java 80.95% <0.00%> (-1.58%) ⬇️
...eduler/api/service/impl/DataSourceServiceImpl.java 29.29% <0.00%> (-0.20%) ⬇️
...duler/api/service/impl/EnvironmentServiceImpl.java 80.51% <ø> (ø)
...scheduler/api/service/impl/ProjectServiceImpl.java 57.92% <ø> (ø)
...heduler/api/service/impl/TaskGroupServiceImpl.java 67.40% <ø> (ø)
...scheduler/api/service/impl/UdfFuncServiceImpl.java 71.63% <ø> (ø)
...duler/server/master/exception/MasterException.java 0.00% <0.00%> (ø)
.../server/master/metrics/ProcessInstanceMetrics.java 59.37% <0.00%> (-3.96%) ⬇️
...r/server/master/runner/MasterSchedulerService.java 0.00% <0.00%> (ø)
.../server/master/runner/StateWheelExecuteThread.java 0.46% <0.00%> (-0.01%) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7898f41...872e4f9. Read the comment docs.

@SbloodyS SbloodyS added backend improvement make more easy to user or prompt friendly labels Jul 1, 2022
@SbloodyS SbloodyS added this to the 3.0.0-beta-3 milestone Jul 1, 2022
lenboo
lenboo previously approved these changes Jul 4, 2022
View reviewed changes
Copy link
Contributor

@lenboo lenboo left a comment

Choose a reason for hiding this comment

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

+1

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

36.7% 36.7% Coverage
0.0% 0.0% Duplication

ruanwenjun
ruanwenjun requested changes Jul 5, 2022
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Please check the all transaction you added

...r-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkerGroupServiceImpl.java Show resolved Hide resolved
@ruanwenjun
Copy link
Member

Note: This PR will introduce some transaction bug, should be fixed at #10747

@caishunfeng caishunfeng merged commit d2fe16d into apache:dev Jul 5, 2022
@WangJPLeo WangJPLeo mentioned this pull request Jul 5, 2022
Merged
@caishunfeng caishunfeng mentioned this pull request Jul 5, 2022
Merged
zhongjiajie pushed a commit to zhongjiajie/dolphinscheduler that referenced this pull request Jul 5, 2022
@WangJPLeo @zhongjiajie
[Optimization] Config module resource relation fix. (apache#10718)
e4e7db1 
* Config module resource relation fix.

* Boolean judge fix

* e2e rerun

* e2e rerun

* e2e rerun

* ArrayList fix

* change variable name.

* Add transaction processing on create data resource method
ruanwenjun pushed a commit to ruanwenjun/dolphinscheduler that referenced this pull request Jul 12, 2022
@WangJPLeo
Config module resource relation fix. apache#10718
e70e63c
ruanwenjun pushed a commit to ruanwenjun/dolphinscheduler that referenced this pull request Jul 12, 2022
@WangJPLeo
[Optimization] Config module resource relation fix.(apache#10718)
1decc76
@zhongjiajie zhongjiajie modified the milestones: 3.0.0-release, 3.1.0-alpha Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruanwenjun ruanwenjun ruanwenjun approved these changes

@caishunfeng caishunfeng caishunfeng approved these changes

@lenboo lenboo lenboo left review comments

@SbloodyS SbloodyS Awaiting requested review from SbloodyS SbloodyS is a code owner

Assignees

@WangJPLeo WangJPLeo

Labels
backend improvement make more easy to user or prompt friendly
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.

[Optimization] Config module resource relation fix.
7 participants
@WangJPLeo @codecov-commenter @ruanwenjun @caishunfeng @lenboo @zhongjiajie @SbloodyS