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.

Sign up for GitHub

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

Jump to bottom

[projects] fix projectId for incremental prebuilds #5540

Merged
merged 1 commit into from
Sep 6, 2021
Merged

[projects] fix projectId for incremental prebuilds #5540

merged 1 commit into from
Sep 6, 2021

Conversation

AlexTugarev
Copy link
Member

Incremental prebuilds should not inherit the projectId of parent prebuilds. This PR should fix such situations.

@AlexTugarev
Copy link
Member Author

/approve no-issue

@AlexTugarev AlexTugarev requested review from svenefftinge and removed request for corneliusludmann September 6, 2021 09:29
geropl
geropl reviewed Sep 6, 2021
View reviewed changes
components/server/ee/src/workspace/workspace-factory.ts
@@ -126,11 +127,11 @@ export class WorkspaceFactoryEE extends WorkspaceFactory {
commit: commitContext.revision,
state: "queued",
creationTime: new Date().toISOString(),
projectId: project?.id,
projectId: ws.projectId,
Copy link
Member

Choose a reason for hiding this comment

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

👍

geropl
geropl reviewed Sep 6, 2021
View reviewed changes
components/server/ee/src/workspace/workspace-factory.ts
@@ -117,6 +117,7 @@ export class WorkspaceFactoryEE extends WorkspaceFactory {
ws = await this.createForCommit({span}, user, commitContext, normalizedContextURL);
}
ws.type = "prebuild";
ws.projectId = project?.id;
Copy link
Member

Choose a reason for hiding this comment

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

I would still argue that this belongs into createForPrebuiltWorkspace (as optional parameter) but that's not too important here, and this fixes it as well.

geropl
geropl approved these changes Sep 6, 2021
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

👍

@roboquat roboquat added the lgtm label Sep 6, 2021
@geropl
Copy link
Member

geropl commented Sep 6, 2021

/lgtm cancel

To unblock our merge queue

@roboquat roboquat removed the lgtm label Sep 6, 2021
@geropl
Copy link
Member

geropl commented Sep 6, 2021

@AlexTugarev small fix: https://werft.gitpod-dev.com/job/gitpod-build-at-prebuild-id.0#validate-changes:validate-changes

@JanKoehnlein
Copy link
Contributor

/lgtm cancel

@roboquat roboquat added size/S and removed size/XS labels Sep 6, 2021
@AlexTugarev
Copy link
Member Author

AlexTugarev commented Sep 6, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Build issue is solved 🤞🏻
@JanKoehnlein good to go?

@geropl
Copy link
Member

geropl commented Sep 6, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Build issue is solved 🤞🏻

Still broken, sadly :-(

@AlexTugarev
Copy link
Member Author

@geropl sorry, the git push -f went to nirvana. had to start a new workspace first... now it's done.

@geropl
Copy link
Member

geropl commented Sep 6, 2021

/lgtm

@roboquat roboquat added the lgtm label Sep 6, 2021
@roboquat
Copy link
Contributor

roboquat commented Sep 6, 2021

LGTM label has been added.

Git tree hash: fc63fe332c0b9ab72d352a211854256a6b6f84f0

@roboquat
Copy link
Contributor

roboquat commented Sep 6, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlexTugarev, geropl

Associated issue requirement bypassed by: AlexTugarev

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 806553f into main Sep 6, 2021
@roboquat roboquat deleted the at/prebuild-id branch September 6, 2021 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geropl geropl geropl approved these changes

@svenefftinge svenefftinge Awaiting requested review from svenefftinge

Assignees

@JanKoehnlein JanKoehnlein

@geropl geropl

Labels
approved size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AlexTugarev @geropl @JanKoehnlein @roboquat