[Fix-17767][Master] fix execute task in workflow instance not effective#18000
[Fix-17767][Master] fix execute task in workflow instance not effective#18000Mrhs121 wants to merge 3 commits intoapache:devfrom
Conversation
Mrhs121
requested review from
SbloodyS,
caishunfeng and
ruanwenjun
as code owners
| // All out varPool from the successors will be merged into the var pool of the task instance | ||
| private List<Property> generateTaskInstanceVarPool(TaskDefinition taskDefinition, | ||
| // The predecessors of the task instance will be used to generate the var pool. | ||
| // In execute-task(TASK_ONLY) scenario, the predecessor might be outside current execution sub-graph. |
There was a problem hiding this comment.
https://github.com/apache/dolphinscheduler/blob/8ee334fc2f4803d7a628458018d82ca695b920d1/dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java#L1152~L1172
The changes here are based on the old behavior in version 3.2
Mrhs121
changed the title
| // The predecessors of the task instance will be used to generate the var pool. | ||
| // In execute-task(TASK_ONLY) scenario, the predecessor might be outside current execution sub-graph. | ||
| // For this case, fallback to workflow varPool to keep compatibility with historical behavior. | ||
| private List<Property> generateTaskInstanceVarPool(WorkflowInstance workflowInstance, |
There was a problem hiding this comment.
It's better to use a clear logic, if the task instance is the start node, then use the workflow instance's varpool as the task instance varpool.
There was a problem hiding this comment.
Good idea. I’ve changed it to explicit start-node logic only start node uses workflow instance varPool
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| .orElseThrow(() -> new IllegalArgumentException("Cannot find WorkflowInstance:" + workflowInstanceId)); | ||
| workflowInstance.setStateWithDesc(WorkflowExecutionStatus.RUNNING_EXECUTION, command.getCommandType().name()); | ||
| workflowInstance.setCommandType(command.getCommandType()); | ||
| if (command.getTaskDependType() != null) { |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| workflowInstance.setStateWithDesc(WorkflowExecutionStatus.RUNNING_EXECUTION, command.getCommandType().name()); | ||
| workflowInstance.setCommandType(command.getCommandType()); | ||
| if (command.getTaskDependType() != null) { | ||
| workflowInstance.setTaskDependType(command.getTaskDependType()); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| .trimResults() | ||
| .omitEmptyStrings() | ||
| .splitToStream(startNodes) | ||
| .map(Long::parseLong) |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
|
3 participants
Was this PR generated or assisted by AI?
Purpose of the pull request
close #17767
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
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