ruanwenjun commented on code in PR #17887:
URL:
https://github.com/apache/dolphinscheduler/pull/17887#discussion_r2704991189
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkflowDefinitionServiceImpl.java:
##########
@@ -428,6 +428,10 @@ private List<WorkflowTaskLineage>
generateWorkflowLineageList(List<TaskDefinitio
.parseObject(taskDefinitionLog.getTaskParams(),
DependentParameters.class)
.getDependence().getDependTaskList()) {
for (DependentItem dependentItem :
dependentTaskModel.getDependItemList()) {
+ // A Dependent node cannot rely on itself workflow
+ if (dependentItem.getDefinitionCode() ==
workflowDefinitionCode) {
+ throw new
ServiceException(Status.WORKFLOW_NODE_HAS_CYCLE);
+ }
Review Comment:
NIT: Maybe only check the dependeny type is `DEPENDENT_ON_WORKFLOW`? I am
not clear if we shouldn't support dependent a task is in the same workflow.
And it's better to provide a method to check if the workflow exist cycle,
since there are some extra case may stiil cause cycle, e.g. dependent a
subworkflow node, the subworkflow contains a subworkflow task point to the
parent workflow.
```suggestion
if (dependentItem.getDependentType() ==
DependentType.DEPENDENT_ON_WORKFLOW && dependentItem.getDefinitionCode() ==
workflowDefinitionCode) {
throw new
ServiceException(Status.WORKFLOW_NODE_HAS_CYCLE);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
