feat: Add default field in parameters.valueFrom #2500
Conversation
| @@ -1801,7 +1801,12 @@ func getTemplateOutputsFromScope(tmpl *wfv1.Template, scope *wfScope) (*wfv1.Out | |||
| } | |||
| val, err := scope.resolveParameter(param.ValueFrom.Parameter) | |||
| if err != nil { | |||
| return nil, err | |||
| // We have a default value to use instead of returning an error | |||
| if param.ValueFrom.Default != "" { | |||
There was a problem hiding this comment.
I've added an E2E test for this.
| } | ||
| } else { | ||
| log.Infof("Copying %s from from volume mount", param.ValueFrom.Path) | ||
| mountedPath := filepath.Join(common.ExecutorMainFilesystemDir, param.ValueFrom.Path) | ||
| out, err := ioutil.ReadFile(mountedPath) | ||
| if err != nil { | ||
| return err | ||
| // We have a default value to use instead of returning an error | ||
| if param.ValueFrom.Default != "" { |
There was a problem hiding this comment.
I didn't include tests for this because it would be difficult to mock this behavior and I have already included a non-E2E test that covers the same behavior here: https://github.com/argoproj/argo/pull/2500/files#diff-4ed679a69dc8fdd934fb1e7f7c0f3259R109-R139
If you think it's necessary, I can invest the time into mocking this or creating an e2e test
| if exErr, ok := err.(*exec.ExitError); ok { | ||
| log.Errorf("`%s` stderr:\n%s", cmd.Args, string(exErr.Stderr)) | ||
| // We have a default value to use instead of returning an error | ||
| if param.ValueFrom.Default != "" { |
There was a problem hiding this comment.
I didn't include tests for this because it would be difficult to mock this behavior and I have already included a non-E2E test that covers the same behavior here: https://github.com/argoproj/argo/pull/2500/files#diff-4ed679a69dc8fdd934fb1e7f7c0f3259R109-R139
If you think it's necessary, I can invest the time into mocking this or creating an e2e test
Codecov Report
@@ Coverage Diff @@
## master #2500 +/- ##
=========================================
Coverage ? 11.36%
=========================================
Files ? 74
Lines ? 31158
Branches ? 0
=========================================
Hits ? 3540
Misses ? 27143
Partials ? 475
Continue to review full report at Codecov.
|
test/e2e/functional_test.go
Outdated
| }) | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
File is not goimports-ed with -local github.com/argoproj/argo (from goimports)
Co-Authored-By: Bot from GolangCI <[email protected]>
|
Will this be / could this be ported to the 2.7 release branch? |
|
@danxmoran Yup, that's the plan! |
Closes #2494 and #2495.
Adds field
defaultinparameters.valueFromthat specifies a default value to be used when retrieving the otherwise specified value fails.(Thanks @samath117!)