feat: Updating workspace prompts new parameters #2598
Conversation
|
I might spend some time trying to refactor this and template update to share param prompt code. |
| @@ -53,4 +80,8 @@ func update() *cobra.Command { | |||
| return nil | |||
| }, | |||
| } | |||
|
|
|||
| cmd.Flags().BoolVar(&alwaysPrompt, "always-prompt", false, "Always prompt all parameters. Does not pull parameter values from existing workspace") | |||
There was a problem hiding this comment.
--reprompt sounds better. Or we could keep it called --always-prompt and ask the user if they want to edit their parameters using a y/n prompt.
There was a problem hiding this comment.
Use --always-prompt and a y/n prompt?
I am not sold on always-prompt either, was unsure on what exactly to use 🤔
There was a problem hiding this comment.
I meant --always-prompt, but if not specified ask them if they want to change values with a y/n prompt
There was a problem hiding this comment.
Ahh. I was trying to keep the flow quick.
I think we need to invest time into thinking about our UX in general. Idk if we should do coder edit or coder configure to adjust params, but we probably need some command UX to manage params in general.
There was a problem hiding this comment.
I can add this prompt though if always-prompt is omitted 👍
There was a problem hiding this comment.
Our y/n prompt only returns yes or error. We need to modify it to accept a proper y/n if we want to do something other than abort on no.
| @@ -37,6 +37,8 @@ type CreateWorkspaceBuildRequest struct { | |||
| Transition WorkspaceTransition `json:"transition" validate:"oneof=create start stop delete,required"` | |||
| DryRun bool `json:"dry_run,omitempty"` | |||
| ProvisionerState []byte `json:"state,omitempty"` | |||
| // ParameterValues are optional. It will write params to the 'workspace' scope. | |||
There was a problem hiding this comment.
You should explain the behavior that it doesn't delete the old values, but any present values will overwrite any existing values with the same name.
There was a problem hiding this comment.
This is true... I wonder if I should delete the unused params 🤔. I can add this in another PR and test it more
| parameters, err := prepWorkspaceBuild(cmd, client, prepWorkspaceBuildArgs{ | ||
| Template: template, | ||
| ExistingParams: existingParams, | ||
| ParameterFile: parameterFile, | ||
| NewWorkspaceName: workspace.Name, | ||
| }) |
There was a problem hiding this comment.
It would be nice if this always ran and had an argument to prompt for missing params or not instead of only running if they prompted for params. We'd be able to always dry-run this way
There was a problem hiding this comment.
You mean instead of passing "ExistingParams", detect that inside the prepWorkspaceBuild? Cause if so, I do want to do this. Templates use similar logic, so I want to refactor the create/update of both workspaces/templates to use the same code.
I was going to take a look in another PR.
What this does
Prompts users if they update a workspace that requires new parameters.
Optionally always prompt params and force updating even if not out of date.
#2463