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

feat: Allow retry strategy on non-leaf nodes, eg for step groups. Fixes #1891 #1892

Merged
merged 3 commits into from Feb 28, 2020

Conversation

markterm
Copy link
Contributor

@markterm markterm commented Dec 26, 2019

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234". Why? for the release notes.
  • Optional. I've added My organization is added to the README.
  • I've signed the CLA and required builds are green.

Fixes #1891

@codecov
Copy link

codecov bot commented Dec 26, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@cdbc619). Click here to learn what that means.
The diff coverage is 72.22%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1892   +/-   ##
=========================================
  Coverage          ?   11.43%           
=========================================
  Files             ?       71           
  Lines             ?    27903           
  Branches          ?        0           
=========================================
  Hits              ?     3192           
  Misses            ?    24305           
  Partials          ?      406
Impacted Files Coverage Δ
workflow/validate/validate.go 74.57% <ø> (ø)
workflow/controller/operator.go 60.57% <72.22%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdbc619...a4f72b2. Read the comment docs.

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

It's looking pretty good at the moment. Could you please look into writing some tests?

@markterm
Copy link
Contributor Author

Sure - are there any specific scenarios you're thinking of other than the one I already created?

@alexec
Copy link
Contributor

alexec commented Jan 2, 2020

@jessesuen do you want to review this one? Or at least provide commentary on the impact on removing this constraint?

@alexec alexec changed the title Allow retry strategy on non-leaf nodes, eg for step groups. Fixes #1891 feat: Allow retry strategy on non-leaf nodes, eg for step groups. Fixes #1891 Jan 3, 2020
@simster7 simster7 added this to the v2.6 milestone Feb 19, 2020
@alexec alexec removed this from the v2.6 milestone Feb 20, 2020
@alexec
Copy link
Contributor

alexec commented Feb 20, 2020

@mark9white I've tried to fix the conflict - but I think I've failed to do a good job - if you still want this - do you want to take a look?

@markterm
Copy link
Contributor Author

@alexec I've fixed the conflict

@alexec alexec added this to the v2.7 milestone Feb 28, 2020
@alexec alexec merged commit e9e13d4 into argoproj:master Feb 28, 2020
@agilgur5 agilgur5 added the area/retryStrategy Template-level retryStrategy label Apr 25, 2024
@argoproj argoproj locked as resolved and limited conversation to collaborators Apr 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/retryStrategy Template-level retryStrategy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable retryStrategy to be specified for non-leaf nodes
6 participants