[Bug] [Workflow] Fix workflow failure strategy bug

[EricGao888]

Purpose of the pull request

• This PR closes: [Bug] [Workflow] Workflow failure strategy does not work as expected #11781

Brief change log

• Make Failure Strategy continue work.

• For details, see [Bug] [Workflow] Workflow failure strategy does not work as expected #11781

Verify this pull request

• Verified by unit tests and manual tests.

[SbloodyS]

LGTM

[codecov-commenter]

Codecov Report

Merging #11793 (69bd990) into dev (4283cfd) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                dev   #11793      +/-   ##
============================================
- Coverage     39.60%   39.58%   -0.02%     
  Complexity     4693     4693              
============================================
  Files          1014     1014              
  Lines         37952    37960       +8     
  Branches       4243     4245       +2     
============================================
- Hits          15030    15027       -3     
- Misses        21315    21326      +11     
  Partials       1607     1607              

Impacted Files	Coverage Δ
.../server/master/runner/WorkflowExecuteRunnable.java	7.96% <0.00%> (-0.01%)	⬇️
...erver/master/processor/queue/TaskEventService.java	75.00% <0.00%> (-5.36%)	⬇️
...che/dolphinscheduler/api/python/PythonGateway.java	17.22% <0.00%> (-0.60%)	⬇️
...ler/server/worker/processor/TaskKillProcessor.java	0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[stalary]

LGTM

[caishunfeng]

I found the UT case WorkflowExecuteRunnableTest.testCheckSerialProcess, can you add a UT case for it?

[EricGao888]

I found the UT case WorkflowExecuteRunnableTest.testCheckSerialProcess, can you add a UT case for it?

@caishunfeng Of course. I will take a look. Thanks for the reminder : )

[ruanwenjun]

LGTM, it will be better if you can add UT.

[EricGao888]

LGTM, it will be better if you can add UT.

Sure, I'm working on it.

[EricGao888]

Refactored some code for better readability and testability. Related unit tests added : ) PTAL @ruanwenjun @caishunfeng @SbloodyS @stalary

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

59.3% Coverage
0.0% Duplication

[SbloodyS]

+1

[caishunfeng]

LGTM 👍

[lenboo]

According to the docs about this feature, I think there are disagreements.

[EricGao888]

According to the docs about this feature, I think there are disagreements.

@lenboo May I ask whether the continue strategy was designed only for parallel execution, or there is an incorrect description in docs? If the latter, I will fix the docs. If the former, we need further discussion. However, IMHO, I think continue strategy should not only work for parallel. Sometimes we need clean up tasks. As described in issue.

[lenboo]

According to the docs about this feature, I think there are disagreements.

@lenboo May I ask whether the continue strategy was designed only for parallel execution, or there is an incorrect description in docs? If the latter, I will fix the docs. If the former, we need further discussion. However, IMHO, I think continue strategy should not only work for parallel. Sometimes we need clean up tasks. As described in issue.

• Yes, the continue strategy was designed only for parallel tasks.
• I can understand the scenario you mentioned. Indeed, we need solution to this scenario in another discussion.

[EricGao888]

According to the docs about this feature, I think there are disagreements.

@lenboo May I ask whether the continue strategy was designed only for parallel execution, or there is an incorrect description in docs? If the latter, I will fix the docs. If the former, we need further discussion. However, IMHO, I think continue strategy should not only work for parallel. Sometimes we need clean up tasks. As described in issue.

• Yes, the continue strategy was designed only for parallel tasks.
• I can understand the scenario you mentioned. Indeed, we need solution to this scenario in another discussion.

@lenboo Thanks for the reply. I've add the topic to community bi-weekly conference topic list, looking forward to the discussion tonight : )

[EricGao888]

I'm closing it temporarily. Will submit a design later.