feat: assign empty value to ephemeral, required parameter while stopping #8545
Conversation
This file contains hidden or 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
johnstcn
reviewed
Jul 17, 2023
| } | ||
|
|
||
| // ValidateResolve checks the provided value, v, against the parameter, p, and the previous build. If v is nil, it also | ||
| // resolves the correct value. It returns the value of the parameter, if valid, and an error if invalid. | ||
| func (r *ParameterResolver) ValidateResolve(p TemplateVersionParameter, v *WorkspaceBuildParameter) (value string, err error) { | ||
| // Assume value for required, ephemeral parameter for stopped/deleted workspace | ||
| if p.Ephemeral && r.Transition != "" && r.Transition != WorkspaceTransitionStart { |
There was a problem hiding this comment.
In what situation will we get an empty transition?
There was a problem hiding this comment.
I guess, only unit tests?
johnstcn
approved these changes
Jul 17, 2023
|
We decided to make |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
… workspace
Related: #6828
Ephemeral parameters are still rich parameters, so they must undergo the same business logic rules.
Parameters with required values (= no default provided) force the workspace owner to provide these values before starting or updating the workspace (if the new template revision introduces a new parameter). Whenever it is possible the value is pulled from the previous workspace build.
Ephemeral parameters don't pull values from previous builds, so in terms of required parameters, user input is obligatory on every workspace build. This PR bends this rule by assuming an empty value for "stop" and "delete" workspace builds.
Side note:
@bpmct It is a product question: maybe it is a better idea to mark properties
required=trueandephemeral=trueas conflicting. So far we identified only one business case:Currently, only properties
mutable=falseandephemeral=trueare marked as conflicting.