feat: cli: allow editing template metadata #2159
Conversation
| minAutostartInterval = time.Duration(template.MinAutostartInterval) | ||
| } | ||
|
|
||
| if err := s.UpdateTemplateMetaByID(r.Context(), database.UpdateTemplateMetaByIDParams{ |
There was a problem hiding this comment.
Question: Do we actually want to mutate the templates here, or should we update the template to a new revision? Motivation:
- Mutation does not leave a trace of the change
- It can become hard to explain how changes in template metadata affect (or don't affect) running workspaces
There was a problem hiding this comment.
Mutation does not leave a trace of the change
Audit logging will cover this, I think
It can become hard to explain how changes in template metadata affect (or don't affect) running workspaces
I would argue strongly that template metadata changes should never affect running workspaces. However, if a template owner edits max_ttl or min_autostart_duration then edits to workspaces, running or not, will be affected.
Additionally, as things stand this would require moving the fields max_ttl and min_autostop_duration_ms to the template_versions table, with all of the associated plumbing.
There was a problem hiding this comment.
Audit logging will cover this, I think
Ok, that's great! As long as there's a trace I think that's good enough.
I would argue strongly that template metadata changes should never affect running workspaces. However, if a template owner edits
max_ttlormin_autostart_durationthen edits to workspaces, running or not, will be affected.
This sounds like it would be fine, for now at least. This is probably the behavior the template editor is looking for. Would someone editing a workspace be able to observe the change when they make the edit? If not that part might be a bit unexpected but otherwise OK IMO.
Additionally, as things stand this would require moving the fields
max_ttlandmin_autostop_duration_msto thetemplate_versionstable, with all of the associated plumbing.
That sounds like a bigger issue and not something we want to do in this PR.
There was a problem hiding this comment.
Would someone editing a workspace be able to observe the change when they make the edit?
Depends if they have the permission to view the template, I guess 🤔 which should be true as far as I know.
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
We do not, apparently |
| count := uint32(0) | ||
| var updated database.Template | ||
| err := api.Database.InTx(func(s database.Store) error { | ||
| // Fetch workspace counts | ||
| workspaceCounts, err := s.GetWorkspaceOwnerCountsByTemplateIDs(r.Context(), []uuid.UUID{template.ID}) | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| if xerrors.Is(err, sql.ErrNoRows) { |
There was a problem hiding this comment.
Hmm, should we add some linting for this? The actual xerrors.Is function is marked deprecated, and we have a lot of mixed usage in the code-base:
❯ rg xerrors.Is | wc -l
20
❯ rg '[^x]errors.Is' | wc -l
104
(Note: I'm not suggesting we fix the usage in this PR).
There was a problem hiding this comment.
AFAIK there are Reasons for using that instead of the original; something about stacktraces.
There was a problem hiding this comment.
Yeah, there was a case for using Errorf and New at least, to get the stack trace. But the others are deprecated and are likely not updated in case there are any changes to stdlib. Interestingly, I now noticed Errorf was also marked deprecated in favor of fmt.Errorf even though the latter does not include a stack trace. Weird.
There was a problem hiding this comment.
Yeah, might be a good topic for dicussion!
There was a problem hiding this comment.
@mafredri we can add a linter for it easy if we want to use xerrors.As/Is over errors.As/Is
m.Match("errors.$f($*_)").
Report("Use xerrors.$f to provide additional stacktrace information!")This PR adds a CLI command template edit which allows updating the following metadata fields of a template: - Description - Max TTL - Min Autostart Interval
This PR adds a CLI command
template editwhich allows updating the following metadata fields of a template: