feat: add inactivity cleanup and failure cleanup configuration fields to Template Schedule Form #7402
Conversation
…plate-route/kira-pilot
…plate-route/kira-pilot
Let's set "7 days" as a default for failure cleanup when enabled. And "180 days" for inactivity cleanup. It's pretty arbitrary, but here's my thinking:
And an admin can always tweak these, but I'd rather default to something more conservative and allow the admin to add more aggressive settings. After all, its disabled by default. |
site/src/pages/TemplateSettingsPage/TemplateSchedulePage/TemplateScheduleForm.tsx
Show resolved
Hide resolved
| }, nil | ||
| } | ||
|
|
||
| func (*enterpriseTemplateScheduleStore) SetTemplateScheduleOptions(ctx context.Context, db database.Store, tpl database.Template, opts schedule.TemplateScheduleOptions) (database.Template, error) { | ||
| if int64(opts.DefaultTTL) == tpl.DefaultTTL && | ||
| int64(opts.MaxTTL) == tpl.MaxTTL && | ||
| int64(opts.FailureTTL) == tpl.FailureTTL && | ||
| int64(opts.InactivityTTL) == tpl.InactivityTTL && |
There was a problem hiding this comment.
@deansheather I see us making what looks like a duplicate check in the patchTemplateMeta handler - curious why we need to check at all (I don't see us doing this in other routes) and also why we need to check twice, once in the handler and once in the store.
There was a problem hiding this comment.
You're right this does seem unnecessary and can probably be removed from the handlers and just kept in the store. Make sure the AGPL version of the store has the equivalent check though (but only for the AGPL-permitted fields)!
There was a problem hiding this comment.
Great, will handle that in a follow-up!
| }, nil | ||
| } | ||
|
|
||
| func (*enterpriseTemplateScheduleStore) SetTemplateScheduleOptions(ctx context.Context, db database.Store, tpl database.Template, opts schedule.TemplateScheduleOptions) (database.Template, error) { | ||
| if int64(opts.DefaultTTL) == tpl.DefaultTTL && | ||
| int64(opts.MaxTTL) == tpl.MaxTTL && | ||
| int64(opts.FailureTTL) == tpl.FailureTTL && | ||
| int64(opts.InactivityTTL) == tpl.InactivityTTL && |
There was a problem hiding this comment.
You're right this does seem unnecessary and can probably be removed from the handlers and just kept in the store. Make sure the AGPL version of the store has the equivalent check though (but only for the AGPL-permitted fields)!
…wn.sql Co-authored-by: Dean Sheather <dean@deansheather.com>
….sql Co-authored-by: Dean Sheather <dean@deansheather.com>
Resolves #7346 and resolves #7347.
#7401 will be a fast follow.Spoke with Ben - we will not be adding these fields to the new template form at the moment.Run locally via
CODER_EXPERIMENTS='workspace_actions' ./scripts/develop.shSome notes:
workspace_actionsfeature. I feel like for now, this feature can live under thecodersdk.FeatureAdvancedTemplateSchedulingentitlement. It is still feature flagged separately with a new experiment (workspace_actions)I am using 14 days as a default value for both "Failure Cleanup" and "Inactivity Cleanup". @bpmct does this seem reasonable?Changed to 7 and 180 respectively as per Ben's comment.Demo
Loom Link
Auditing works, too: