feat: cli: consolidate schedule-related commands #2402

johnstcn · 2022-06-16T11:20:24Z

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 list
WORKSPACE                       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 cian
Starts 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 manual
Starts at    manual  
Starts next  -       
Stops at     manual  
Stops next   -       

$ coderv2 schedule override-stop cian 4h
Starts 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)

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

…command

johnstcn · 2022-06-16T13:32:53Z

Makefile

-          -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


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

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

johnstcn · 2022-06-16T13:33:09Z

cli/constants.go

-	timeFormat = "3:04:05 PM MST"
+	timeFormat = "3:04PM MST"


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

johnstcn · 2022-06-16T13:33:16Z

cli/list.go

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


Clearer language

johnstcn · 2022-06-16T13:33:25Z

cli/list.go

@@ -90,7 +89,7 @@ func list() *cobra.Command {
 				autostartDisplay := "-"
 				if !ptr.NilOrEmpty(workspace.AutostartSchedule) {
 					if sched, err := schedule.Weekly(*workspace.AutostartSchedule); err == nil {
-						autostartDisplay = sched.Cron()
+						autostartDisplay = fmt.Sprintf("%s %s (%s)", sched.Time(), sched.DaysOfWeek(), sched.Location())


Consistent schedule format

johnstcn · 2022-06-16T13:34:30Z

cli/util.go

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


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

johnstcn · 2022-06-16T13:35:11Z

coderd/autobuild/schedule/schedule.go

+// Time returns a humanized form of the minute and hour fields.
+func (s Schedule) 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)
+	if err != nil {
+		// return the original cronspec for minute and hour, who knows what's in there!
+		return fmt.Sprintf("cron(%s %s)", minute, hour)
+	}
+	return t.Format(time.Kitchen)
+}


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.

johnstcn · 2022-06-16T14:20:47Z

cli/list.go

-					if has, 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))


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

johnstcn · 2022-06-16T14:43:26Z

cli/util.go

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


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

ammario · 2022-06-16T14:54:22Z

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.
+`
+)


👍 Solid descriptions

ammario · 2022-06-16T14:54:48Z

cli/schedule.go

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


all great help 👍

cli/schedule.go

ammario · 2022-06-16T15:00:43Z

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.
+`


All these newlines at the beginning of the help should be removed.

cli/schedule.go

mafredri

Lovely changes!

cli/schedule.go

mafredri · 2022-06-16T15:03:59Z

cli/schedule.go

+
+			loc, err := tz.TimezoneIANA()
+			if err != nil {
+				loc = time.UTC // best effort


Yeah, we tried... ❤️

cli/schedule.go

cli/schedule_test.go

cli/util.go

…command

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 added 8 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 from a team June 16, 2022 14:36

johnstcn commented Jun 16, 2022

View reviewed changes

ammario requested changes Jun 16, 2022

View reviewed changes

mafredri approved these changes Jun 16, 2022

View reviewed changes

johnstcn added 3 commits June 16, 2022 16:00

address PR comments

81b2656

address PR comments

dc16686

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

aced9da

…command

ammario approved these changes Jun 16, 2022

View reviewed changes

johnstcn merged commit c9691ea into main Jun 16, 2022

johnstcn deleted the cj/2258/cli-schedule-command branch June 16, 2022 17:24

This was referenced Jun 16, 2022

cli: collect all schedule-related commands under coder schedule #2258

Closed

Bug: coder ls output should measure TTL from the time the provision job was completed #2197

Closed

Unify all workspace information in coder show #1646

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: cli: consolidate schedule-related commands #2402

feat: cli: consolidate schedule-related commands #2402

johnstcn commented Jun 16, 2022 •

edited

Loading

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

johnstcn Jun 16, 2022

ammario Jun 16, 2022

ammario Jun 16, 2022

ammario Jun 16, 2022

mafredri left a comment

mafredri Jun 16, 2022

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

feat: cli: consolidate schedule-related commands #2402

feat: cli: consolidate schedule-related commands #2402

Conversation

johnstcn commented Jun 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mafredri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnstcn commented Jun 16, 2022 •

edited

Loading