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-16978][Master] Fix AbstractDelayEvent compare method is incorrect #16980

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

[Fix-16978][Master] Fix AbstractDelayEvent compare method is incorrect #16980

wants to merge 8 commits into from
+271 −12

Conversation

reele
Copy link
Contributor

@reele reele commented Jan 23, 2025 •
edited
Loading

Purpose of the pull request

Fix AbstractDelayEventBus block events issue fix #16978.

Brief change log

  1. Fix AbstractDelayEvent compare method is incorrect.
  2. Modify TaskFailureStateAction to allow killEventAction when task instance can retry.
  3. modify WorkflowInstanceTopTestCase and WorkflowInstancePauseTestCase, add affected workflow stop and pause with failed retrying task test cases.
  4. fix (I)WorkflowExecutionGraph.isTaskExecutionRunnableActive method is incorrect.

Verify this pull request

Stop a workflow with failed task when waiting to retry.

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

@SbloodyS SbloodyS added the bug Something isn't working label Jan 23, 2025
@SbloodyS SbloodyS added this to the 3.3.0 milestone Jan 23, 2025
@ruanwenjun
Copy link
Member

BTW, we are missing test cases in WorkflowInstanceStopTestCase, it's better to add a case kill a workflow which contains retry task.

@reele
Copy link
Contributor Author

reele commented Jan 23, 2025

BTW, we are missing test cases in WorkflowInstanceStopTestCase, it's better to add a case kill a workflow which contains retry task.

sure, i'll do that.

@github-actions github-actions bot added the test label Jan 25, 2025
@reele
Copy link
Contributor Author

reele commented Jan 25, 2025

@ruanwenjun
Test cases added, and passed all test in master and worker.

ruanwenjun
ruanwenjun previously approved these changes Jan 26, 2025
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.

High quality PR, looks pretty good.

@reele
Copy link
Contributor Author

reele commented Jan 26, 2025 •
edited
Loading

High quality PR, looks pretty good.

@ruanwenjun
thx, there is a new commit to remove the redundant code

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
53.8% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

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 Awaiting requested review from caishunfeng caishunfeng is a code owner

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

At least 2 approving reviews are required to merge this pull request.

Assignees

@reele reele

Labels
backend bug Something isn't working priority:high test
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

[Bug] [Master] Fix AbstractDelayEvent compare method is incorrect
3 participants
@reele @ruanwenjun @SbloodyS