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

Refactor Manager StartWorkspace #11547

Merged
merged 1 commit into from
Jul 22, 2022
Merged

Refactor Manager StartWorkspace #11547

merged 1 commit into from
Jul 22, 2022

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Jul 21, 2022

Description

The call startWorkspace here uses a timeout of 30s that is not respected here and the consequence is that we never remove the annotation gitpod.io/attemptingToCreate and ws-daemon never creates the socket in the workspace.

Similar to #11327

This is also the place that introduces the behavior of why some workspaces stay in Preparing workspace … and after a refresh, we can open the IDE.

With this change, we don't see the behavior in the workspace cluster, but we still require a refresh to open the IDE.

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@roboquat roboquat added size/M and removed size/S labels Jul 22, 2022
@aledbf aledbf marked this pull request as ready for review July 22, 2022 14:00
@aledbf aledbf requested a review from a team July 22, 2022 14:00
@aledbf aledbf requested a review from sagor999 as a code owner July 22, 2022 14:00
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Jul 22, 2022
@aledbf aledbf requested a review from geropl July 22, 2022 14:00
@@ -117,16 +117,17 @@ export class PromisifiedWorkspaceManagerClient implements Disposable {
this.client.startWorkspace(
request,
withTracing({ span }),
this.getDefaultUnaryOptions(),
{
// Important!!!!: client timeout must be higher than ws-manager to be able to process any error
Copy link
Member

Choose a reason for hiding this comment

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

🧡

@roboquat roboquat merged commit d0e6bd7 into main Jul 22, 2022
@roboquat roboquat deleted the aledbf/start branch July 22, 2022 14:07
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.

Code LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/M team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants