Skip to content
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

[server, cli] Allow flexible workspace timeouts #15815

Merged
merged 1 commit into from
Jan 18, 2023

Conversation

svenefftinge
Copy link
Member

@svenefftinge svenefftinge commented Jan 18, 2023

Description

Makes setWorkspaceTimout handle arbitrary poistive duration strings, matching the format <\D+[mhd]>, which is compatible with go's time Duration.

Related Issue(s)

Fixes #9038

How to test

  • Verify that extending timouts still work as before
  • try gp timeout set 4h

Release Notes

A new CLI command `gp timeout set` allows to set the workspace timeout to arbitrary durations.

Documentation

Build Options:

  • /werft with-github-actions
    Experimental feature to run the build with GitHub Actions (and not in Werft).
  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-payment
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-se-allow-flexible-timeouts.1 because the annotations in the pull request description changed
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the se/allow-flexible-timeouts branch from 7fa4b30 to 5ff0ac7 Compare January 18, 2023 08:48
@svenefftinge svenefftinge force-pushed the se/allow-flexible-timeouts branch 2 times, most recently from 4096151 to cce772f Compare January 18, 2023 09:00
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-se-allow-flexible-timeouts.5 because the annotations in the pull request description changed
(with .werft/ from main)

@svenefftinge svenefftinge marked this pull request as ready for review January 18, 2023 12:06
@svenefftinge svenefftinge requested review from a team January 18, 2023 12:06
@github-actions github-actions bot added team: IDE team: webapp Issue belongs to the WebApp team labels Jan 18, 2023
@jldec
Copy link
Contributor

jldec commented Jan 18, 2023

Tested in preview

gitpod /workspace/readme (main) $ gp timeout show
Timeout for current workspace is 30m
gitpod /workspace/readme (main) $ gp timeout extend
Cannot extend workspace timeout for current plan, please upgrade your plan
## 
## had to add PAYG before being allowed to set timeout (this was expected)
## 
gitpod /workspace/readme (main) $ gp timeout set 180
jsonrpc2: code 650 message: Invalid duration : Invalid timeout unit: 0
gitpod /workspace/readme (main) $ gp help timeout  set
Set timeout of current workspace.

Duration must be in the format of <n>m (minutes), <n>h (hours), or <n>d (days).
For example, 30m, 1h, 2d, etc.

Usage:
  gp timeout set <duration> [flags]

Examples:
gitpod timeout set 1h

Flags:
  -h, --help   help for set

$ gp timeout show
Timeout for current workspace is 30m

$ gp timeout extend
Workspace timeout has been extended to three hours.

$ gp timeout set 300m
Workspace timeout has been set to 300m.

$ gp timeout set 5h
Workspace timeout has been set to 5h.

$ gp timeout show
Timeout for current workspace is 5h

It would be better if the response to gp timeout set and gp timeout show did not use the same input string as was entered via gp timeout set because this created uncertainty about whether the input was parsed correctly.

Suggested interaction

  • consistent use of "n minutes" in responses (no period at the end)
  • maybe simpler error messages
$ gp timeout set 300m
Workspace timeout has been set to 300 minutes

$ gp timeout show
Timeout for current workspace is 300 minutes

$ gp timeout set 5h
Workspace timeout has been set to 300 minutes

$ gp timeout show
Timeout for current workspace is 300 minutes

$ gp timeout set 1d
Workspace timeout has been set to 1440 minutes

$ gp timeout set 1d5h
Error: invalid duration "1d5h": time: unknown unit "d" in duration "1d5h"

@svenefftinge svenefftinge force-pushed the se/allow-flexible-timeouts branch 3 times, most recently from 8cac780 to 4c1de74 Compare January 18, 2023 14:07
@svenefftinge
Copy link
Member Author

I've added the suggested changes.

@svenefftinge svenefftinge changed the title [server] Allow flexible workspace timeouts [server, cli] Allow flexible workspace timeouts Jan 18, 2023
@easyCZ
Copy link
Member

easyCZ commented Jan 18, 2023

/hold for review Qs

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

Code wise looks ok.

components/gitpod-cli/cmd/timeout-show.go Outdated Show resolved Hide resolved
components/gitpod-cli/cmd/timeout-show.go Outdated Show resolved Hide resolved
@svenefftinge svenefftinge force-pushed the se/allow-flexible-timeouts branch from 4c1de74 to c79ff26 Compare January 18, 2023 14:58
@svenefftinge
Copy link
Member Author

/unhold

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

Code looks good, rely on @svenefftinge for testing. I did not try.

/hold

merge when you are ready 🚀

@svenefftinge
Copy link
Member Author

/unhold

@roboquat roboquat merged commit 95996df into main Jan 18, 2023
@roboquat roboquat deleted the se/allow-flexible-timeouts branch January 18, 2023 15:14
@roboquat roboquat added deployed: IDE IDE change is running in production deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: gp cli deployed: IDE IDE change is running in production deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/L team: IDE team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Epic: User Configurable Workspace Timeouts (CLI command + user preference)
6 participants