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

[dashboard]: Use prebuild record when determining prebuild actions #8841

Merged
merged 1 commit into from
Apr 8, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Mar 16, 2022

Description

Actions shown on Prebuild Detail view are now based on the prebuild record, rather than a Workspace Instance.

Screenshot 2022-04-05 at 19 40 19

Screenshot 2022-04-05 at 19 40 07

Screenshot 2022-04-05 at 19 39 56

Screenshot 2022-04-05 at 19 39 51

Visual remain unchanged, only logic for showing various buttons is updated.

Related Issue(s)

Relates to #8197

How to test

  1. Setup a project with https://gitlab.com/gitpod-milan/gitpod-large-image
  2. Trigger prebuilds for the following branches:
    • pass
    • timeout
    • fail-prebuild-fast
    • fail-prebuild (and cancel the prebuild)
  3. Observe appropriate buttons shown on prebuild detail view

Release Notes

Prebuild Detail view buttons are based on Prebuild status, instead of WorkspaceInstance

/hold

@stale
Copy link

stale bot commented Mar 26, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Mar 26, 2022
@easyCZ easyCZ removed the meta: stale This issue/PR is stale and will be closed soon label Mar 26, 2022
@easyCZ easyCZ force-pushed the mp/prebuild-logs branch from 5573078 to dcdd384 Compare March 30, 2022 12:13
@roboquat roboquat added size/L and removed size/XS labels Mar 31, 2022
@easyCZ easyCZ force-pushed the mp/prebuild-logs branch from 06501ca to 658ea60 Compare April 5, 2022 08:29
@roboquat roboquat added size/M and removed size/L labels Apr 5, 2022
@easyCZ easyCZ force-pushed the mp/prebuild-logs branch from 658ea60 to 10c43eb Compare April 5, 2022 08:30
@roboquat roboquat added size/L and removed size/M labels Apr 5, 2022
@easyCZ easyCZ force-pushed the mp/prebuild-logs branch from 7b7af36 to 6fe99fd Compare April 5, 2022 14:22
@roboquat roboquat added size/M and removed size/L labels Apr 5, 2022
@easyCZ easyCZ force-pushed the mp/prebuild-logs branch 2 times, most recently from 85a3f2f to ae559bf Compare April 5, 2022 14:31
@easyCZ easyCZ changed the title WIP: Normalize prebuild logs view status [dashboard]: Use prebuild record when determining prebuild actions Apr 5, 2022
@easyCZ
Copy link
Member Author

easyCZ commented Apr 5, 2022

/werft run

@easyCZ easyCZ marked this pull request as ready for review April 5, 2022 19:29
@easyCZ easyCZ requested a review from a team April 5, 2022 19:29
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Apr 5, 2022
@@ -19,7 +19,6 @@ const WorkspaceLogs = React.lazy(() => import("./WorkspaceLogs"));

export interface PrebuildLogsProps {
workspaceId?: string;
onInstanceUpdate?: (instance: WorkspaceInstance) => void;
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed as the change to Prebuild.tsx was the only remaining usage.

@easyCZ
Copy link
Member Author

easyCZ commented Apr 6, 2022

/werft run

@easyCZ easyCZ force-pushed the mp/prebuild-logs branch 3 times, most recently from 80967a0 to f444087 Compare April 6, 2022 13:16
@@ -56,6 +55,16 @@ export default function () {
});
setPrebuild(prebuilds[0]);
})();

return getGitpodService().registerClient({
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of previously receiving updates through the onInstanceUpdate (which came from a child component), we now receive the prebuild updates directly here.

Copy link
Member

Choose a reason for hiding this comment

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

Note that we have a gap here on reconnects: To make this bullet-proof we'd need to detect reconnects, and actively fetch the current state afterwards to ensure we stay up-to-date (there is a utility that does so for instance-updates built into the TS client, but I don't think there is one for the prebuild).

However let's not block here, but maybe take a note/create a follow-up issues - this is already much better than the status quo.

@easyCZ easyCZ marked this pull request as ready for review April 6, 2022 13:23
@easyCZ easyCZ force-pushed the mp/prebuild-logs branch 2 times, most recently from 9ffa7ac to 59244a0 Compare April 7, 2022 06:38
@easyCZ
Copy link
Member Author

easyCZ commented Apr 7, 2022

/werft run with-clean-slate-deployment=true

👍 started the job as gitpod-build-mp-prebuild-logs.24

@easyCZ easyCZ force-pushed the mp/prebuild-logs branch from 59244a0 to 7152484 Compare April 7, 2022 13:14
@easyCZ
Copy link
Member Author

easyCZ commented Apr 7, 2022

/werft run with-clean-slate-deployment=true

@geropl geropl marked this pull request as draft April 7, 2022 13:37
@geropl
Copy link
Member

geropl commented Apr 7, 2022

@easyCZ converted to draft bc I saw a recent push, and am trying to follow my own "PR reviews" script 🙄 👍 ... feedback welcome! 💯

@easyCZ
Copy link
Member Author

easyCZ commented Apr 7, 2022

@geropl The push is a rebase to try and get image builds working on the preview environment as suggested in internal slack.

@easyCZ easyCZ marked this pull request as ready for review April 7, 2022 13:40
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 merged commit 19fe0d3 into main Apr 8, 2022
@roboquat roboquat deleted the mp/prebuild-logs branch April 8, 2022 07:04
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/M team: webapp Issue belongs to the WebApp team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants