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

[public-api] add v2 WorkspaceService.getWorkspace #18930

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Oct 16, 2023

Description

Summary generated by Copilot

🤖 Generated by Copilot at 446315d

This pull request adds the proto definition and the generated code for the WorkspaceService, a new experimental API for managing workspaces in Gitpod. It also removes a dependency on the gitpod-protocol library from the public-api typescript package, to avoid a circular dependency and a conflict with the existing typescript definitions. The generated code includes Connect and gRPC clients and servers in Go and typescript, as well as proxy handlers and service types. The WorkspaceService is part of a larger effort to improve the user experience and performance of Gitpod, by providing more granular and flexible control over workspaces and their resources.

Related Issue(s)

Fixes #

How to test

Documentation

Preview status

gitpod:summary

Build Options

Build
  • /werft with-werft
    Run the build with werft instead of GHA
  • leeway-no-cache
  • /werft no-test
    Run Leeway with --dont-test
Publish
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer
  • analytics=segment
  • with-dedicated-emulation
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
  • with-monitoring

/hold

@geropl
Copy link
Member

geropl commented Oct 17, 2023

Taking a look now 👀


message GetWorkspaceRequest { string id = 1; }

message GetWorkspaceResponse { Workspace item = 1; }
Copy link
Member

Choose a reason for hiding this comment

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

nit: Has there been a discussion on the use of item as generic name for the contained element in the response?
Alternatively, one could name it specific to the resource type (workspace).

// and user.
//
// +optional
repeated WorkspaceEnvironmentVariable additional_environment_variables = 7;
Copy link
Member

Choose a reason for hiding this comment

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

☁️ All the following fields are part of the "spec", but also mirrored back as part of the "status". We could think about bringing this even closer to the kubernetes-style, and on the top level fully separate the two. But I think that's out of scope here, just leaving it if we think about this again int he future. 👍

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.

Proto LGTM 👍

It's hard to judge how well this works in practice, but I think you tried that already. 😉

Also, we could discuss endlessly on minor details - but this seems like a very good evolutional step of what we have, so: Let's go! 🚀

@akosyakov
Copy link
Member Author

/unhold

@roboquat roboquat merged commit 2338184 into main Oct 17, 2023
@roboquat roboquat deleted the ak/get_workspace_v2 branch October 17, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants