NotificationsYou must be signed in to change notification settings
Fork1k
Star11.2k

feat: cli: consolidate schedule-related commands#2402

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

johnstcn merged 12 commits intomainfromcj/2258/cli-schedule-command

Jun 16, 2022

Merged

feat: cli: consolidate schedule-related commands#2402

johnstcn merged 12 commits intomainfromcj/2258/cli-schedule-command

Jun 16, 2022

Conversation

Copy link

Member

johnstcn commentedJun 16, 2022•
edited
Loading

This commit makes the following changes:

cil: rename autostart -> schedule start
cli: rename ttl -> schedule stop
cli: rename bump -> schedule override
cli: add schedule show command
cli: move some cli-related stuff to util.go
cli: modify list command output slightly to show time remaining

Sample output:

$ coderv2 listWORKSPACE                       TEMPLATE      STATUS   LAST BUILT  OUTDATED  STARTS AT                         STOPS AFTER  johnstcn/cian                   coder         Running   6h23m       false     9:00AM Mon-Fri (Europe/Dublin)    12h (in 5h3m)    johnstcn/cian-test              coder         Stopped   1h33m       false     -                                 -                johnstcn/cian-test-linux        gcp-linux     Failed    17h17m      false     -                                 12h     $ coderv2 schedule show cianStarts at    9:00AM Mon-Fri (Europe/Dublin)        Starts next  9:00AM IST on Jun 17, 2022            Stops at     12h after start                       Stops next   7:27PM UTC on Jun 16, 2022 (in 5h3m)  $ coderv2 schedule stop cian-test manualStarts at    manual  Starts next  -       Stops at     manual  Stops next   -       $ coderv2 schedule override-stop cian 4hStarts at    9:00AM Mon-Fri (Europe/Dublin)         Starts next  9:00AM IST on Jun 17, 2022             Stops at     12h after start                        Stops next   6:24PM UTC on Jun 16, 2022 (in 3h59m)

feat: cli: consolidate schedule-related commands

70cec9d

This commit makes the following changes:- renames autostart -> schedule starat- renames ttl -> schedule stop- renames bump -> schedule override- adds schedule show command- moves some cli-related stuff to util.go

johnstcn self-assigned this

Jun 16, 2022

johnstcn added8 commits

June 16, 2022 11:53

fixup! feat: cli: consolidate schedule-related commands

b2e8f15

Makefile: test-postgres: increase timeout and parallelism

8fa27aa

make lint

ae6c047

more tests, some cli tweaks

f817751

update schedule override output to match other commands

f6f8de0

show time until deadline

dad6695

bump test timeout

bf1bafd

Merge remote-tracking branch 'origin/main' into cj/2258/cli-schedule-…

3441dab

…command

johnstcn marked this pull request as ready for review

June 16, 2022 14:36

johnstcn requested a review froma team

June 16, 2022 14:36

johnstcn commented

Jun 16, 2022

View reviewed changes

Makefile Outdated

Comment on lines 118 to 120

		-covermode=atomic -coverprofile="gotests.coverage" -timeout=5m\
		-covermode=atomic -coverprofile="gotests.coverage" -timeout=10m\
		-coverpkg=./...,github.com/coder/coder/codersdk\
		-count=1 -parallel=1 -race -failfast
		-count=1 -parallel=2 -race -failfast

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

👀 bumping these here because we are absolutely hitting the five-minute mark

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

Spike's PR (#2413) actually addresses this better so I'm going to revert these changes.

cli/constants.go


		const (
		timeFormat="3:04:05 PM MST"
		timeFormat="3:04PM MST"

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

I don't think seconds are useful to users here.

cli/list.go

Comment on lines -53 to +52

		header:= table.Row{"workspace","template","status","last built","outdated","autostart","ttl"}
		header:= table.Row{"workspace","template","status","last built","outdated","starts at","stops after"}

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

Clearer language

cli/list.go

		if!ptr.NilOrEmpty(workspace.AutostartSchedule) {
		ifsched,err:=schedule.Weekly(*workspace.AutostartSchedule);err==nil {
		autostartDisplay=sched.Cron()
		autostartDisplay=fmt.Sprintf("%s %s (%s)",sched.Time(),sched.DaysOfWeek(),sched.Location())

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

Consistent schedule format

cli/util.go

		@@ -0,0 +1,207 @@
		package cli

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

There were some helper methods about the place that I decided to pull into here.

coderd/autobuild/schedule/schedule.go

Comment on lines +142 to +153

		// Time returns a humanized form of the minute and hour fields.
		func (sSchedule)Time()string {
		minute:=strings.Fields(s.cronStr)[0]
		hour:=strings.Fields(s.cronStr)[1]
		maybeTime:=fmt.Sprintf("%s:%s",hour,minute)
		t,err:=time.ParseInLocation("3:4",maybeTime,s.sched.Location)
		iferr!=nil {
		// return the original cronspec for minute and hour, who knows what's in there!
		returnfmt.Sprintf("cron(%s %s)",minute,hour)
		}
		returnt.Format(time.Kitchen)
		}

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

This is a best-effort of showing the scheduled time of a schedule. If we can't display it as a simple time, we just return the cron for it.

cli/list.go

Comment on lines -101 to +103

		ifhas,ext:=hasExtension(workspace);has {
		autostopDisplay+=fmt.Sprintf(" (+%s)",durationDisplay(ext.Round(time.Minute)))
		if!workspace.LatestBuild.Deadline.IsZero()&&workspace.LatestBuild.Deadline.After(now)&&status=="Running" {
		remaining:=time.Until(workspace.LatestBuild.Deadline)
		autostopDisplay=fmt.Sprintf("%s (%s)",autostopDisplay,relative(remaining))

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

Felt it's more useful to show the time remaining here

johnstcn commented

Jun 16, 2022

View reviewed changes

cli/util.go

Comment on lines 26 to 60

		funcdurationDisplay(d time.Duration)string {
		duration:=d
		sign:=""
		ifduration==0 {
		return"0s"
		}
		ifduration<0 {
		duration*=-1
		}
		// duration > 0 now
		ifduration<time.Minute {
		returnsign+"<1m"
		}
		ifduration>24*time.Hour {
		duration=duration.Truncate(time.Hour)
		}
		ifduration>time.Minute {
		duration=duration.Truncate(time.Minute)
		}
		days:=0
		forduration.Hours()>=24 {
		days++
		duration-=24*time.Hour
		}
		durationDisplay:=duration.String()
		ifdays>0 {
		durationDisplay=fmt.Sprintf("%dd%s",days,durationDisplay)
		}
		for_,suffix:=range []string{"m0s","h0m","d0s"} {
		ifstrings.HasSuffix(durationDisplay,suffix) {
		durationDisplay=durationDisplay[:len(durationDisplay)-2]
		}
		}
		returnsign+durationDisplay
		}

Copy link

MemberAuthor

johnstcnJun 16, 2022

Choose a reason for hiding this comment

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

review: smaller durations are always <1m now, it can also handle negative durations

ammario requested changes

Jun 16, 2022

View reviewed changes

cli/schedule.go

		* The new stop time must be at least 30 minutes in the future.
		* The workspace template may restrict the maximum workspace runtime.
		`
		)

Copy link

Member

ammarioJun 16, 2022

Choose a reason for hiding this comment

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

👍 Solid descriptions

cli/schedule.go

		Use:"stop <workspace_name> { <duration> \| manual }",
		Example:`stop my-workspace 2h30m`,
		Short:"Edit workspace stop schedule",
		Long:scheduleStopDescriptionLong,

Copy link

Member

ammarioJun 16, 2022

Choose a reason for hiding this comment

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

all great help 👍

cli/schedule.go OutdatedShow resolvedHide resolved

cli/schedule.go

		* The new stop time is calculated from now.
		* The new stop time must be at least 30 minutes in the future.
		* The workspace template may restrict the maximum workspace runtime.
		`