kfp-tekton icon indicating copy to clipboard operation
kfp-tekton copied to clipboard

fix(sdk): update loop dsl to handle same argument in nested loop

Open Tomcli opened this issue 3 years ago • 3 comments

Which issue is resolved by this Pull Request: Resolves #1045

Description of your changes: The original loop dsl only returns the original argument name no matter the loop is nested or not, updating loop dsl to handle same argument in nested loops.

Environment tested:

  • Python Version (use python --version):
  • Tekton Version (use tkn version):
  • Kubernetes Version (use kubectl version):
  • OS (e.g. from /etc/os-release):

Checklist:

Tomcli avatar Sep 30 '22 21:09 Tomcli

cc @Udiknedormin I have to change the way how the dsl name the argument in order to fix this issue. Let me know if you see any problem with this approach.

Tomcli avatar Sep 30 '22 22:09 Tomcli

@ScrapCodes for the sdk/python/tests/compiler/testdata/nested_loop_same_arg.yaml example, the pipeline loop cache hit triggered despite the argument is different in the next nested loop. I think your caching logic was based on the spec and and argument name, but the nested loop could have the same argument name but passing down a different value. So we may need a better way to check for this edge case as well.

Tomcli avatar Sep 30 '22 22:09 Tomcli

[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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

google-oss-prow[bot] avatar Oct 24 '22 21:10 google-oss-prow[bot]