Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: regression in memoization without outputs #12130

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

Joibel
Copy link
Member

@Joibel Joibel commented Nov 3, 2023

#11379 allowed memoization without outputs.

The merge with #11451 reverted this, so this commit is just to reinstate that.
The tests included in #11379 failed to catch this, I've raised #12129 to fix this, but in the interests of matching the documentation and with time limited and kubecon next week I'm putting this PR in now.

Fixes #12117

The merge with argoproj#11451 reverted this, so this commit is just to
reinstate that.
The tests included in argoproj#11379 failed to catch this, I've raised argoproj#12129
for this, but in the interests of matching the documentation and
kubecon next week I'm putting this PR in now.

Fixes argoproj#12117

Signed-off-by: Alan Clucas <alan@clucas.org>
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that there was a subtle accidental revert on this line due to the merge and that this change reinstates it

@terrytangyuan terrytangyuan merged commit c751e66 into argoproj:master Nov 3, 2023
28 checks passed
terrytangyuan pushed a commit that referenced this pull request Nov 3, 2023
Signed-off-by: Alan Clucas <alan@clucas.org>
terrytangyuan pushed a commit that referenced this pull request Nov 3, 2023
Signed-off-by: Alan Clucas <alan@clucas.org>
terrytangyuan added a commit that referenced this pull request Nov 15, 2023
Joibel added a commit to Joibel/argo-workflows that referenced this pull request Nov 15, 2023
This is a partial revert of argoproj#12130 and fixes argoproj#12165

Offered as an alternative to a full revert.

Signed-off-by: Alan Clucas <alan@clucas.org>
Joibel added a commit to Joibel/argo-workflows that referenced this pull request Nov 15, 2023
This is a partial revert of argoproj#12130 and fixes argoproj#12165

Offered as an alternative to a full revert.

Signed-off-by: Alan Clucas <alan@clucas.org>
Joibel added a commit to Joibel/argo-workflows that referenced this pull request Nov 15, 2023
This is a partial revert of argoproj#12130 and fixes argoproj#12165

Offered as an alternative to a full revert

Signed-off-by: Alan Clucas <alan@clucas.org>
terrytangyuan added a commit that referenced this pull request Nov 15, 2023
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
Signed-off-by: Alan Clucas <alan@clucas.org>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
@Joibel Joibel deleted the memoize-regression branch May 9, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Template level memoization requires outputs
3 participants