Skip to content

feat: cli: consolidate schedule-related commands#2402

Merged
johnstcn merged 12 commits into
mainfrom
cj/2258/cli-schedule-command
Jun 16, 2022
Merged

feat: cli: consolidate schedule-related commands#2402
johnstcn merged 12 commits into
mainfrom
cj/2258/cli-schedule-command

Conversation

@johnstcn
Copy link
Copy Markdown
Member

@johnstcn johnstcn commented Jun 16, 2022

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
@johnstcn johnstcn self-assigned this Jun 16, 2022
@johnstcn johnstcn marked this pull request as ready for review June 16, 2022 14:36
@johnstcn johnstcn requested a review from a team June 16, 2022 14:36
Comment thread 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
Copy Markdown
Member Author

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
Copy Markdown
Member Author

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.

Comment thread cli/constants.go

const (
timeFormat = "3:04:05 PM MST"
timeFormat = "3:04PM MST"
Copy link
Copy Markdown
Member Author

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.

Comment thread 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
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Clearer language

Comment thread cli/list.go
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())
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Consistent schedule format

Comment thread cli/util.go
@@ -0,0 +1,207 @@
package cli
Copy link
Copy Markdown
Member Author

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.

Comment on lines +142 to +153
// 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)
}
Copy link
Copy Markdown
Member Author

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.

Comment thread cli/list.go
Comment on lines -101 to +103
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))
Copy link
Copy Markdown
Member Author

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

Comment thread cli/util.go
Comment on lines +26 to +60
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
}
Copy link
Copy Markdown
Member Author

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

Comment thread 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
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 Solid descriptions

Comment thread cli/schedule.go
Use: "stop <workspace_name> { <duration> | manual }",
Example: `stop my-workspace 2h30m`,
Short: "Edit workspace stop schedule",
Long: scheduleStopDescriptionLong,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

all great help 👍

Comment thread cli/schedule.go Outdated
Comment thread cli/schedule.go Outdated
Comment thread 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.
`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread cli/schedule.go
Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Lovely changes!

Comment thread cli/schedule.go Outdated
Comment thread cli/schedule.go Outdated
Comment thread cli/schedule.go Outdated
Comment thread cli/schedule.go Outdated
Comment thread cli/schedule.go

loc, err := tz.TimezoneIANA()
if err != nil {
loc = time.UTC // best effort
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, we tried... ❤️

Comment thread cli/schedule.go Outdated
Comment thread cli/schedule_test.go Outdated
Comment thread cli/util.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants