Skip to content

[Gitpod CLI] gp rebuild improvements #15740

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

Merged
merged 1 commit into from
Jan 18, 2023
Merged

[Gitpod CLI] gp rebuild improvements #15740

merged 1 commit into from
Jan 18, 2023

Conversation

andreafalzetti
Copy link
Contributor

@andreafalzetti andreafalzetti commented Jan 14, 2023

Description

Follow up PR of things we overlooked in the initial PR:

  1. add event outcome,
  2. use the tmp dockerfile
  3. build from checkout location context (using . won't work if the user is inside a sub-directory)
  4. the Stderr coming from docker run should not be propagated outside of the container because it can be anything (e.g user typing exit 1) which currently gets logged as an error from gp rebuild
  5. return an explicit nil error when there is not real error
success event user error system error
Screenshot 2023-01-17 at 22 27 53 Screenshot 2023-01-17 at 22 27 44 Screenshot 2023-01-17 at 22 30 40

Related Issue(s)

Fixes #

How to test

  1. Start a workspace
  2. Create a Dockerfile which used COPY and relies on location, make sure files are copied correctly from the workspace checkout location
  3. Try using an image which does not have bash installed (ie. busybox)
  4. Try typing exit 1 to leave the container
  5. test each branch changed

Release Notes

NONE

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh
  • /werft analytics=segment

@werft-gitpod-dev-com

This comment was marked as resolved.

@andreafalzetti andreafalzetti mentioned this pull request Jan 14, 2023
5 tasks
@roboquat roboquat added size/M and removed size/S labels Jan 14, 2023
@andreafalzetti andreafalzetti marked this pull request as ready for review January 14, 2023 01:06
@andreafalzetti andreafalzetti requested a review from a team January 14, 2023 01:06
@akosyakov

This comment was marked as resolved.

@roboquat roboquat added size/L and removed size/M labels Jan 18, 2023
Co-authored-by: Victor Nogueira <[email protected]>
@werft-gitpod-dev-com

This comment was marked as resolved.

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 wise it seems to be alright, I have not tried

/hold

@andreafalzetti @felladrin please merge when you are sure

Copy link
Contributor

@felladrin felladrin left a comment

Choose a reason for hiding this comment

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

Reviewed and tested! ✅

@roboquat roboquat merged commit 75fec4a into main Jan 18, 2023
@roboquat roboquat deleted the fix/gp-rebuild branch January 18, 2023 09:24
@roboquat roboquat added deployed: IDE IDE 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 Change is completely running in production release-note-none size/L team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants