kvermeulen commented on code in PR #17578:
URL:
https://github.com/apache/dolphinscheduler/pull/17578#discussion_r2495879434
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/executor/plugin/subworkflow/SubWorkflowLogicTask.java:
##########
@@ -234,7 +249,7 @@ private SubWorkflowLogicTaskRuntimeContext
triggerNewSubWorkflow() {
.tenantCode(workflowInstance.getTenantCode())
.environmentCode(workflowInstance.getEnvironmentCode())
// todo: transport varpool and local params
- .startParamList(commandParam.getCommandParams())
+
.startParamList(mergeParams(asList(commandParam.getCommandParams(),
globalParamsOfParents)))
Review Comment:
Sorry, I missed that indeed. Parent global params already exist in the
command params. I'll update and remove the unnecessary code. Can you maybe
verify if the order of merging the params is correct? Before I didn't include
the varpool for instance.
I'll update my commit and force push on the PR. I don't know if this is best
practice or if we'll loose trace of the previous change. However, I do like a
code changes for a fix in a single commit. Makes it more concise maybe. But any
advice here is also welcome.
--
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]
