fix(backend): Update compiler with new test sdk and fix dag publisher execution id #1425
Conversation
|
Using the logic of root dag publisher, publisher's parent dag should be the driver dag execution id. Thus we need to update that logic. |
|
/assign @yhwang |
| @@ -378,7 +378,7 @@ func (i *pubDagDriverInputs) getParentDagID(isExitHandler bool) string { | |||
| if isExitHandler && i.parentDagID == compiler.RootComponentName { | |||
| return fmt.Sprintf("$(params.%s)", paramParentDagID) | |||
| } else if i.inLoopDag { | |||
| return fmt.Sprintf("$(params.%s)", paramParentDagID) | |||
| return taskOutputParameter(getDAGDriverTaskName(i.parentDagID), paramExecutionID) | |||
There was a problem hiding this comment.
I believe we have 4 cases here,
- if root
- if exit handler root
- if subdag
- else
In all our current test cases, we only trigger the last two scenarios because it doesn't make sense for publisher to be root or exit handler root. We can either keep all these 4 scenarios to be in sync with getParentDagID. Or we can just make the whole function to return taskOutputParameter(getDAGDriverTaskName(i.parentDagID), paramExecutionID).
WDYT? I would just go ahead and check if getParentDagID != "" or nil then return taskOutputParameter else raise error.
There was a problem hiding this comment.
for me, logically, merging them and adding comments there would be good. otherwise, it's confusing.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Tomcli, yhwang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Which issue is resolved by this Pull Request:
Resolves #
Description of your changes:
Environment tested:
python --version):tkn version):kubectl version):/etc/os-release):Checklist: