fix: use env when pod version annotation is missing. Fixes #10237 #10457
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
Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
isubasinghe
changed the title
NikeNano
reviewed
Feb 4, 2023
| @@ -79,8 +79,10 @@ func ensurePodNamePrefixLength(prefix string) string { | |||
| // given workflow | |||
| func GetWorkflowPodNameVersion(wf *v1alpha1.Workflow) PodNameVersion { | |||
| annotations := wf.GetAnnotations() | |||
| version := annotations[common.AnnotationKeyPodNameVersion] | |||
|
|
|||
| version, ok := annotations[common.AnnotationKeyPodNameVersion] | |||
There was a problem hiding this comment.
Is the default in the switch case below still valid? Otherwise this could potentially be rewritten so that GetPodNameVersion() is the default case?
There was a problem hiding this comment.
I was tempted to implement this fix that way but that default switch case will be hit if the annotation is invalid. That is a different behaviour to the GetPodNameVersion call.
But yeah I was on the fence on this about what the "right behaviour" is. This is why I went with the current approach, at least it will always be consistent when the user makes a wrong annotation (default to v2).
alexec
approved these changes
Feb 12, 2023
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
33 tasks
terrytangyuan
pushed a commit
that referenced
this pull request
Mar 29, 2023
5 tasks
7 tasks
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.
Signed-off-by: Isitha Subasinghe isitha@pipekit.io
Fixes #10237
Please do not open a pull request until you have checked ALL of these:
make pre-commit -Bto fix codegen and lint problems.If changes were requested, and you've made them, dismiss the review to get it reviewed again.