Skip to content

feat: allow templates to specify max_ttl or autostop_requirement #10920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Dec 15, 2023

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Nov 28, 2023

  • Allow both max_ttl and autostop_requirement at the same time in deployments
  • Default to CRON_TZ=UTC 0 0 * * * for quiet hours schedule (so it's always enabled if entitled, can be overridden with flag still)
  • Removes feature flag template_autostop_requirement, merged into advanced_template_scheduling

TODO:

  • API for template to select which one to use
  • Show both in UI, only let one be set at a time
Create template form
chrome_8fiabxzSQz.mp4
Edit template form
chrome_UcH8t1ESW6.mp4

Closes #9284

@deansheather deansheather marked this pull request as draft November 28, 2023 15:39
@deansheather deansheather marked this pull request as ready for review December 11, 2023 12:46
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BE is good, just need to remove a line to make tests work

MaxTTL time.Duration `json:"max_ttl"`
// UseMaxTTL dictates whether the max_ttl should be used instead of
// autostop_requirement for this template. This is governed by the template
// and licensing.
// TODO(@dean): remove this when we remove max_tll
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still intend to remove max_ttl? Or remove this comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's deprecated

}
enterpriseTemplateStore.UseAutostopRequirement.Store(true)

quietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(api.DefaultQuietHoursSchedule)
if err != nil {
api.Logger.Error(ctx, "unable to set up enterprise user quiet hours schedule store, template autostop requirements will not be applied to workspace builds", slog.Error(err))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that we have the Health page, it would be cool if we could elevate this kind of stuff to there.

@@ -121,6 +122,7 @@ export const FormSection: FC<
>
{title}
{alpha && <AlphaBadge />}
{deprecated && <DeprecatedBadge />}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you drop a screencap with what this looks like?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@deansheather deansheather merged commit b36071c into main Dec 15, 2023
@deansheather deansheather deleted the dean/max-ttl-comeback branch December 15, 2023 08:27
@github-actions github-actions bot locked and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quiet hours: deprecate and eventually remove max_ttl
3 participants