Skip to content

Add support for setting codespace retention-period - #5504

Merged
mislav merged 2 commits into
cli:trunkfrom
greggroth:gregg/retention-period
May 10, 2022
Merged

Add support for setting codespace retention-period#5504
mislav merged 2 commits into
cli:trunkfrom
greggroth:gregg/retention-period

Conversation

@greggroth

@greggroth greggroth commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

Fixes https://github.com/github/codespaces/issues/7177

The Codespace creation API will soon support setting a retention period where a codespace will be auto-deleted once it's been idle for that amount of time.

@gaowhire gaowhire left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

gh pr checkout 217

@greggroth
greggroth marked this pull request as ready for review April 27, 2022 18:20
@greggroth
greggroth requested a review from a team as a code owner April 27, 2022 18:20
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Apr 27, 2022
Comment thread internal/codespaces/api/api.go Outdated
PermissionsOptOut bool `json:"multi_repo_permissions_opt_out"`
RepositoryID int `json:"repository_id"`
IdleTimeoutMinutes int `json:"idle_timeout_minutes,omitempty"`
RetentionPeriodMinutes int `json:"retention_period_minutes,omitempty"`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@reybard pointed out that this won't serialize a 0-minute retention period with omitempty so I'll add a test and fix that

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just to confirm: the user could specify 0 here, in order to indicate they want to delete it immediately upon idle?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@lostintangent That is correct. It may not be immediate as it's a scheduled job but it should be pretty quick

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Awesome, thanks for confirming that!

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great to me 🚀

With omitempty, it would remove 0 as the empty value of an integer
@mislav
mislav enabled auto-merge (squash) May 10, 2022 16:12
@mislav
mislav disabled auto-merge May 10, 2022 16:21
@mislav
mislav merged commit 0615377 into cli:trunk May 10, 2022
@greggroth

Copy link
Copy Markdown
Contributor Author

We don't have API support for this flag just yet and it's not ready to ship with the CLI. Sorry I didn't make that very clear in the description.

@greggroth
greggroth deleted the gregg/retention-period branch May 10, 2022 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants