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

[Fix #15129] [Dependent] The date rules of the dependent node are ambiguous. #15289

Merged
merged 6 commits into from
Dec 8, 2023
Merged

[Fix #15129] [Dependent] The date rules of the dependent node are ambiguous. #15289

merged 6 commits into from
Dec 8, 2023

Conversation

reele
Copy link
Contributor

@reele reele commented Dec 6, 2023 •
edited
Loading

Purpose of the pull request

This pull request fix #15129

Brief change log

Restore the date matching logic for dependent task in scheduling:
search for processInstance through the scheduleTime in schedule command. Through the processInstance, search for taskInstance. The 'endTime' of the taskInstance is only used to sort and find the maximum value.

Add corresponding indexes for the new SQL definition.

Note: This may cause historical cost logic conflicts with existing users in dev branch.

@SbloodyS

Verify this pull request

This pull request is already covered by existing tests.

@mergeable Mergeable
Copy link

mergeable bot commented Dec 6, 2023

⚠️ This PR do not change database DDL synchronize.

@SbloodyS SbloodyS added bug Something isn't working first time contributor First-time contributor 3.2.1 labels Dec 6, 2023
@SbloodyS SbloodyS added this to the 3.2.1 milestone Dec 6, 2023
@mergeable mergeable bot removed the sql not sync label Dec 6, 2023
@SbloodyS
Copy link
Member

SbloodyS commented Dec 6, 2023 •
edited
Loading

#15129 (comment)

We can add this change to the document in the version 3.2.1 to inform the user of this change. @reele

@reele
Copy link
Contributor Author

reele commented Dec 6, 2023

#15129 (comment)

We can add this change to the document in the version 3.2.1 to inform the user of this change. @reele

Sure!

@SbloodyS SbloodyS added the miss:docs missing documents in PR label Dec 6, 2023
@SbloodyS
Copy link
Member

SbloodyS commented Dec 6, 2023

You can add it to https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/zh/guide/upgrade/incompatible.md. @reele

@reele
Copy link
Contributor Author

reele commented Dec 6, 2023 •
edited
Loading

You can add it to https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/zh/guide/upgrade/incompatible.md.

docs updated. @SbloodyS

@reele reele requested a review from EricGao888 as a code owner December 6, 2023 13:11
@mergeable mergeable bot removed the document label Dec 6, 2023
@github-actions github-actions bot removed the document label Dec 7, 2023
SbloodyS
SbloodyS previously approved these changes Dec 8, 2023
View reviewed changes
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM

@SbloodyS SbloodyS added ready-to-merge and removed miss:docs missing documents in PR labels Dec 8, 2023
SbloodyS
SbloodyS reviewed Dec 8, 2023
View reviewed changes
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

Please run mvn spotless:apply to format code. @reele

@reele
Copy link
Contributor Author

reele commented Dec 8, 2023 •
edited
Loading

Please run mvn spotless:apply to format code. @reele

Command completed. @SbloodyS

@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2023 •
edited
Loading

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (0bb48f3) 38.19% compared to head (b71c6d9) 38.19%.

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

Files Patch % Lines
...cheduler/server/master/utils/DependentExecute.java 0.00% 5 Missing ⚠️
...duler/dao/repository/impl/TaskInstanceDaoImpl.java 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##                dev   #15289      +/-   ##
============================================
- Coverage     38.19%   38.19%   -0.01%     
  Complexity     4673     4673              
============================================
  Files          1278     1278              
  Lines         44481    44482       +1     
  Branches       4783     4783              
============================================
  Hits          16988    16988              
- Misses        25631    25632       +1     
  Partials       1862     1862

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

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Dec 8, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@SbloodyS SbloodyS merged commit 0c470ff into apache:dev Dec 8, 2023
52 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SbloodyS SbloodyS SbloodyS approved these changes

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

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

@zhongjiajie zhongjiajie Awaiting requested review from zhongjiajie

@EricGao888 EricGao888 Awaiting requested review from EricGao888

Assignees

@reele reele

Labels
3.2.1 backend bug Something isn't working first time contributor First-time contributor ready-to-merge
Projects
None yet
Milestone
3.2.1
Development

Successfully merging this pull request may close these issues.

[Bug] [Dependent] The date rules of the dependent node are ambiguous.
3 participants
@reele @SbloodyS @codecov-commenter