Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: set prebuilds lifecycle parameters on creation and claim#19252

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

Merged

Conversation

ssncferreira
Copy link
Contributor

@ssncferreirassncferreira commentedAug 8, 2025
edited
Loading

Description

This PR ensures that prebuilt workspaces are properly excluded from the lifecycle executor and treated as a separate class of workspaces, fully managed by the prebuild reconciliation loop.

It introduces two lifecycle guarantees:

  • When a prebuilt workspace is created (i.e., when the workspace build completes), all lifecycle-related fields are unset, ensuring the workspace does not participate in TTL, autostop, autostart, dormancy, or auto-deletion logic.
  • When a prebuilt workspace is claimed, it transitions into a regular user workspace. At this point, all lifecycle fields are correctly populated according to template-level configurations, allowing the workspace to be managed by the lifecycle executor as expected.

Changes

  • Prebuilt workspaces now have all lifecycle-relevant fields unset during creation
  • When a prebuild is claimed:
  • Lifecycle fields are set based on template and workspace level configurations. This ensures a clean transition into the standard workspace lifecycle flow.
  • Updated lifecycle-related SQL update queries to explicitly exclude prebuilt workspaces.

Relates

Related issue:#18898

To reduce the scope of this PR and make the review process more manageable, the original implementation has been split into the following focused PRs:

These PRs should be considered in conjunction with this one to understand the complete set of lifecycle separation changes for prebuilt workspaces.

@ssncferreirassncferreiraforce-pushed thessncferreira/fix-prebuilds-lifecycle-params branch 4 times, most recently fromec09fd8 todd57747CompareAugust 8, 2025 11:48
@ssncferreirassncferreira changed the titlefix: prebuilds lifecycle parameters on creation and claimfix: set prebuilds lifecycle parameters on creation and claimAug 8, 2025
@ssncferreirassncferreiraforce-pushed thessncferreira/fix-prebuilds-lifecycle-params branch fromdd57747 to6415069CompareAugust 8, 2025 14:34
@ssncferreirassncferreira marked this pull request as ready for reviewAugust 8, 2025 14:34
autostart_schedule= $2,
next_start_at= $3
WHERE
id= $1;
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: make it impossible by addingAND owner_id != ...

Copy link
ContributorAuthor

@ssncferreirassncferreiraAug 8, 2025
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I wasn’t sure at first whether to enforce it at the database level or just in the application layer, but I agree it makes more sense to make it impossible by adding a DB-level constraint.

Addressed in4c847c9 and I’ve updated the other relevant queries as well to align with it.

@ssncferreira
Copy link
ContributorAuthor

As suggested, the scope of this PR was reduced by splitting the original implementation into smaller, more focused PRs.

Comment on lines +521 to +525
id= $1
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
-- are managed by the reconciliation loop, not the lifecycle executor which handles
-- autostart_schedule and next_start_at
AND owner_id!='c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID;
Copy link
Member

Choose a reason for hiding this comment

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

This seems more related to#19264?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

As discussed internally will keep this constraint in this PR as per the PR description:Updated lifecycle-related SQL update queries to explicitly exclude prebuilt workspaces.

Copy link
Member

@johnstcnjohnstcn left a comment

Choose a reason for hiding this comment

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

Approving, but would also like input from@SasSwart before merge

ssncferreira reacted with thumbs up emoji
Copy link
Contributor

@SasSwartSasSwart left a comment

Choose a reason for hiding this comment

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

Thanks Susana. Good work!

@johnstcn I've double checked all your feedback here as requested. None of it seemed controversial and I like the way it has been addressed.

ssncferreira reacted with hooray emoji
@ssncferreirassncferreira merged commit8567ecb intomainAug 13, 2025
26 checks passed
@ssncferreirassncferreira deleted the ssncferreira/fix-prebuilds-lifecycle-params branchAugust 13, 2025 11:45
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 13, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@SasSwartSasSwartSasSwart left review comments

@johnstcnjohnstcnjohnstcn approved these changes

@DanielleMaywoodDanielleMaywoodAwaiting requested review from DanielleMaywood

Assignees

@ssncferreirassncferreira

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@ssncferreira@johnstcn@SasSwart

[8]ページ先頭

©2009-2025 Movatter.jp