SDK - Compiler - Fixed ParallelFor argument resolving #3029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ark-kun
force-pushed
the
SDK---Compiler---Fixed-ParallelFor-name-clashes
branch
2 times, most recently
from
e11070d
to
2992fe9
Compare
The ParallelFor argument reference resolving was really broken. The logic "worked" like this - of the name of the referenced output contained the name of the loop collection source output, then it was considered to be the reference to the loop item. This broke lots of scenarios especially in cases where there were multiple components with same output name (e.g. the default "Output" output name). The logic also did not distinguish between references to the loop collection item vs. references to the loop collection source itself. I've rewritten the argument resolving logic, to fix the issues.
Ark-kun
force-pushed
the
SDK---Compiler---Fixed-ParallelFor-name-clashes
branch
from
2992fe9
to
8bd6620
Compare
Ark-kun
changed the title
Ark-kun
changed the title
Ark-kun
force-pushed
the
SDK---Compiler---Fixed-ParallelFor-name-clashes
branch
2 times, most recently
from
183c9e4
to
16493ab
Compare
Ark-kun
force-pushed
the
SDK---Compiler---Fixed-ParallelFor-name-clashes
branch
from
16493ab
to
5f626c1
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun 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 |
|
/lgtm |
Jeffwan
pushed a commit
to Jeffwan/pipelines
that referenced
this pull request
Dec 9, 2020
* SDK - Compiler - Fixed ParallelFor name clashes
The ParallelFor argument reference resolving was really broken.
The logic "worked" like this - of the name of the referenced output
contained the name of the loop collection source output, then it was
considered to be the reference to the loop item.
This broke lots of scenarios especially in cases where there were
multiple components with same output name (e.g. the default "Output"
output name). The logic also did not distinguish between references to
the loop collection item vs. references to the loop collection source
itself.
I've rewritten the argument resolving logic, to fix the issues.
* Argo cannot use {{item}} when withParams items are dicts
* Stabilize the loop template names
* Renamed the test case
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR rewrites the ParallelFor loop item argument resolving to solve many issues discovered by our users.
The ParallelFor argument reference resolving was really broken.
The old code implemented the following faulty logic: If the name of the referenced output contained the name of the loop collection source output, then it was considered to be the reference to the loop item.
This broke lots of scenarios especially in cases where there were multiple components with same output name (e.g. the default "Output" output name). The logic also did not distinguish between references to the loop collection item vs. references to the loop collection source itself.
I've rewritten the argument resolving logic, to fix the issues.
I need to make couple of checks to verify that everything works as expected.
Fixes #2943
Fixes #2936
Fixes #2569
Fixes #2829
This change is