Skip to content

[Improve] Enhance complement function transformation #10376

Merged
zhongjiajie merged 26 commits intoapache:devfrom
hstdream:bushu_0609
Jun 12, 2022
Merged

[Improve] Enhance complement function transformation #10376
zhongjiajie merged 26 commits intoapache:devfrom
hstdream:bushu_0609

Conversation

@hstdream
Copy link
Copy Markdown
Contributor

@hstdream hstdream commented Jun 8, 2022 •
edited by SbloodyS
Loading

close #10370

SbloodyS
SbloodyS reviewed Jun 9, 2022
View reviewed changes
caishunfeng
caishunfeng reviewed Jun 9, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

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

I think it's not a complete pr and we should add the auto generate date tool on ui.

@SbloodyS SbloodyS added improvement make more easy to user or prompt friendly backend labels Jun 9, 2022
@lenboo
Copy link
Copy Markdown
Contributor

lenboo commented Jun 9, 2022

I think we can keep two complement data way in the same time.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 10, 2022 •
edited
Loading

Codecov Report

Merging #10376 (e13fb00) into dev (b0d9d3f) will decrease coverage by 0.03%.
The diff coverage is 32.57%.

@@             Coverage Diff              @@
##                dev   #10376      +/-   ##
============================================
- Coverage     40.56%   40.53%   -0.04%     
- Complexity     4767     4772       +5     
============================================
  Files           877      877              
  Lines         35623    35701      +78     
  Branches       3945     3970      +25     
============================================
+ Hits          14452    14470      +18     
- Misses        19738    19787      +49     
- Partials       1433     1444      +11
Impacted Files Coverage Δ
...inscheduler/api/controller/ExecutorController.java 28.30% <ø> (ø)
.../org/apache/dolphinscheduler/common/Constants.java 80.95% <ø> (ø)
.../server/master/runner/WorkflowExecuteRunnable.java 7.75% <0.00%> (-0.07%) ⬇️
...pache/dolphinscheduler/service/corn/CronUtils.java 64.77% <0.00%> (-5.60%) ⬇️
...cheduler/api/service/impl/ExecutorServiceImpl.java 40.10% <47.86%> (-2.59%) ⬇️
.../org/apache/dolphinscheduler/api/enums/Status.java 100.00% <100.00%> (ø)
...r/plugin/task/sqoop/parameter/SqoopParameters.java 53.33% <0.00%> (ø)
...er/master/dispatch/host/assign/RandomSelector.java 83.33% <0.00%> (+5.55%) ⬆️

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 b0d9d3f...e13fb00. Read the comment docs.

@hstdream
Because the input parameters of runworkflow are modified, the E2E is …
96dfe2f 
…shielded for the time being. After the front-end code is merged, it is opened again
@hstdream hstdream requested a review from kezhenxu94 as a code owner June 10, 2022 01:19
@hstdream
Copy link
Copy Markdown
Contributor Author

hstdream commented Jun 10, 2022

@SbloodyS @caishunfeng

@hstdream hstdream requested review from SbloodyS and caishunfeng June 10, 2022 03:36
@caishunfeng
Copy link
Copy Markdown
Contributor

caishunfeng commented Jun 11, 2022

LGTM overall. But we still have discussion in #10376 (comment)

Hi @SbloodyS you should give a approve because the change requested.

Sorry. I don't have strong opinion here. I just not quite sure about whether should give a approve since we still have unresolved discussions.

I gave approved. Do you have better suggestions? @zhongjiajie @caishunfeng

Sorry, I got it wrong, thanks for pointing it out.

caishunfeng
caishunfeng previously approved these changes Jun 11, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

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

+1

@zhongjiajie
Copy link
Copy Markdown
Member

zhongjiajie commented Jun 11, 2022

LGTM overall. But we still have discussion in #10376 (comment)

Hi @SbloodyS you should give a approve because the change requested.

Sorry. I don't have strong opinion here. I just not quite sure about whether should give a approve since we still have unresolved discussions.

I gave approved. Do you have better suggestions? @zhongjiajie @caishunfeng

i think we better add some java doc on that constants value about why we set it to 100 instead of other value

@hstdream
Copy link
Copy Markdown
Contributor Author

hstdream commented Jun 11, 2022

LGTM overall. But we still have discussion in #10376 (comment)

Hi @SbloodyS you should give a approve because the change requested.

Sorry. I don't have strong opinion here. I just not quite sure about whether should give a approve since we still have unresolved discussions.
I gave approved. Do you have better suggestions? @zhongjiajie @caishunfeng

i think we better add some java doc on that constants value about why we set it to 100 instead of other value
At that time, it was considered that the amount of data was too large, and then set the value of 100.

zhongjiajie
zhongjiajie previously approved these changes Jun 12, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@zhongjiajie zhongjiajie left a comment

Choose a reason for hiding this comment

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

LGTM, waittig CI

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jun 12, 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 5 Code Smells

37.8% 37.8% Coverage
0.0% 0.0% Duplication

@zhongjiajie
Copy link
Copy Markdown
Member

zhongjiajie commented Jun 12, 2022

I restart the failing E2E/fileManageCI, and why this test failed often

@zhongjiajie zhongjiajie mentioned this pull request Jun 12, 2022
Closed
3 tasks
@zhongjiajie zhongjiajie removed the miss:docs missing documents in PR label Jun 12, 2022
@zhongjiajie zhongjiajie merged commit f6fea06 into apache:dev Jun 12, 2022
@zhongjiajie zhongjiajie changed the title [Improvement] [Api & Master] Complement function transformation [Improve] Enhance complement function transformation Jun 12, 2022
Tianqi-Dotes pushed a commit to Tianqi-Dotes/dolphinscheduler that referenced this pull request Jun 16, 2022
@hstdream
[Improve] Enhance complement function transformation (apache#10376)
e2c6c6c
Tianqi-Dotes pushed a commit to Tianqi-Dotes/dolphinscheduler that referenced this pull request Jun 16, 2022
@caishunfeng
Merge pull request #2 from WhaleOps/bushu_300
a22a979 
[Improve] Enhance complement function transformation (apache#10376)
@zhongjiajie zhongjiajie added this to the 3.1.0-alpha milestone Jun 23, 2022
@zhongjiajie zhongjiajie mentioned this pull request Jun 23, 2022
Merged
ITBOX-ITBOY pushed a commit to ITBOX-ITBOY/dolphinscheduler that referenced this pull request Jul 8, 2022
@hstdream
[Improve] Enhance complement function transformation (apache#10376)
eb19961
@SbloodyS SbloodyS mentioned this pull request Feb 14, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lenboo lenboo lenboo left review comments

@zhongjiajie zhongjiajie zhongjiajie approved these changes

@caishunfeng caishunfeng caishunfeng left review comments

@SbloodyS SbloodyS SbloodyS left review comments

@WangJPLeo WangJPLeo WangJPLeo left review comments

@kezhenxu94 kezhenxu94 Awaiting requested review from kezhenxu94

@Tianqi-Dotes Tianqi-Dotes Awaiting requested review from Tianqi-Dotes

Assignees

@hstdream hstdream

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.

[Improvement] [Api & Master] Complement function transformation

7 participants

@hstdream @lenboo @codecov-commenter @zhongjiajie @caishunfeng @SbloodyS @WangJPLeo