[public-api] User service #14496

jeanp413 · 2022-11-08T04:53:59Z

Description

Implements user service and remaining methods needed for complete migration in gitpod-desktop extension

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

akosyakov · 2022-11-08T10:51:45Z

@easyCZ Could you have a look whether proposed APIs sound good to you? Or you will model them differently?

components/public-api/gitpod/experimental/v1/user.proto

easyCZ

Broadly, this makes sense.

Worth asking the question whether SSH Keys strictly live on a User level, or if they could live seperately, or on a team. If that's the case, you'd then create a SSHKeyService rather than embed it on the user.

components/public-api/gitpod/experimental/v1/user.proto

easyCZ · 2022-11-08T12:26:44Z

For these definitions, are you looking to implement all of them? Or are you defining them for completeness but would only use a subset of them?

akosyakov

lgtm

@jeanp413 discussed with @easyCZ that you can iterate as you want and ask for guidance, but generally for experimental there are no strict requirements

/hold

please merge when you are comfortable

jeanp413 · 2022-11-11T07:25:10Z

Or are you defining them for completeness but would only use a subset of them?

For the ssh methods only need the list ssh keys, others are just there for completeness

werft-gitpod-dev-com · 2022-11-11T07:26:59Z

started the job as gitpod-build-jp-user-service.4 because the annotations in the pull request description changed
(with .werft/ from main)

easyCZ · 2022-11-11T07:45:56Z

components/public-api/gitpod/experimental/v1/workspaces.proto

+    // SendHeartbeat sends a heartbeat signal to a running workspace.
+    rpc SendHeartbeat(SendHeartbeatRequest) returns (SendHeartbeatResponse) {}
+
+    // SendCloseHeartbeat sends a close heartbeat signal to a running workspace.
+    rpc SendCloseHeartbeat(SendCloseHeartbeatRequest) returns (SendCloseHeartbeatResponse) {}


FWIW, I'm not convinced these should exist on the Public API, at least not on the Workspace Service. Why? Because these are not directly usable by our users, and are mostly an implementation detail of how we communicate with the rest of the system.

I think these are a good use-case for a private API package which still benefits from the API quality aspects, but doesn't actually get exposed to the public users.

For now, we can keep them here but when we move the APIs to the "stable" package, I wouldn't include them.
Maybe we can for now move them under a WorkspaceInstance service, or something closer related to IDE behaviour, rather than a Workspace behaviour and properties. What do you think?

For now we can move it to some experimental service like IDEWorkspaceInstanceService.

In future we would like to allow 3rd party IDEs, instead of dealing with supporting each of it. It is part of our long term motivation in Public API then integrators should be able to report heartbeating and closing.

Got it. Perhaps WorkspaceIDEService makes sense for that use-case

In general, I'd like each service to be as minimal as possible, with additional services possibly layering on top and composing the basic services. Here, I'd see it as IDEWorkspaceService -> WorkspaceService.GetWorkspace -> use that info to do something to the instance for example

easyCZ · 2022-11-11T07:47:57Z

components/public-api/gitpod/experimental/v1/workspaces.proto

+message SendHeartbeatRequest {
+    string workspace_id = 1;
+}


If we extend this struct with, we can avoid having an RPC for every property in our domain model

Suggested change

message SendHeartbeatRequest {

string workspace_id = 1;

}

message SendHeartbeatRequest {

string workspace_id = 1;

// indicates this is the terminal heartbeat and no further ones will be sent. This happens when the IDE closes.

bool terminal = 2;

}

Could you elaborate which property?

We would like to split 2 different functions SendHearbeat and report that a client closed. Before it was mixed in one but turned out to be orthogonal things from client perspective.

If they are orthogonal, then that's fine and should be seperate RPCs. From the implementation it did look like they are coupled but I don't know your domain that well so that makes sense.

easyCZ · 2022-11-11T07:56:33Z

Let's try to land some of this. Ideally, we would split up the PRs into:

Just protos, without any implementation. This ensures that we look at the API unbiased on what's possible and focus solely on the UX of the API
Ideally, 1 PR per API Service to further drive that focus.

Could we remove the changes to the workspace service from this PR (and push into a new one) and we can land this as is with the User Service?

akosyakov · 2022-11-11T08:55:12Z

components/public-api/gitpod/experimental/v1/workspaces.proto

+    rpc SendHeartbeat(SendHeartbeatRequest) returns (SendHeartbeatResponse) {}
+
+    // SendCloseHeartbeat sends a close heartbeat signal to a running workspace.
+    rpc SendCloseHeartbeat(SendCloseHeartbeatRequest) returns (SendCloseHeartbeatResponse) {}


I would call it SendClose or SendCloseSignal. The point is that it is not heartbeating fiction anymore.

akosyakov · 2022-11-11T12:30:13Z

Could we remove the changes to the workspace service from this PR (and push into a new one) and we can land this as is with the User Service?

That's all experimental and there can be follow up issues to address API and implementation? Nobody is using it right now? Should not we rather approve, merge and go via this loop to let @jeanp413 iterate faster?

easyCZ

Sure, let's land as is. But please do follow-up on moving the Heartbeat methods into a different service.

werft-gitpod-dev-com · 2022-11-12T09:05:45Z

started the job as gitpod-build-jp-user-service.8 because the annotations in the pull request description changed
(with .werft/ from main)

jeanp413 · 2022-11-12T09:20:59Z

/werft run

👍 started the job as gitpod-build-jp-user-service.9
(with .werft/ from main)

jeanp413 · 2022-11-12T09:34:24Z

/unhold

roboquat added release-note-none do-not-merge/work-in-progress size/L labels Nov 8, 2022

jeanp413 changed the title ~~User service~~ [public-api] User service Nov 8, 2022

akosyakov reviewed Nov 8, 2022

View reviewed changes

components/public-api/gitpod/experimental/v1/user.proto Outdated Show resolved Hide resolved

easyCZ reviewed Nov 8, 2022

View reviewed changes

akosyakov approved these changes Nov 8, 2022

View reviewed changes

roboquat added do-not-merge/hold size/XL size/XXL and removed size/L size/XL labels Nov 8, 2022

jeanp413 marked this pull request as ready for review November 11, 2022 07:25

jeanp413 requested review from a team November 11, 2022 07:25

jeanp413 requested a review from csweichel as a code owner November 11, 2022 07:25

roboquat removed the do-not-merge/work-in-progress label Nov 11, 2022

github-actions bot added team: SID team: IDE team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Nov 11, 2022

easyCZ reviewed Nov 11, 2022

View reviewed changes

akosyakov reviewed Nov 11, 2022

View reviewed changes

easyCZ approved these changes Nov 11, 2022

View reviewed changes

ArthurSens approved these changes Nov 11, 2022

View reviewed changes

aledbf approved these changes Nov 11, 2022

View reviewed changes

User service

12731f6

jeanp413 force-pushed the jp/user-service branch from 54d24a0 to 12731f6 Compare November 12, 2022 08:26

roboquat removed the do-not-merge/hold label Nov 12, 2022

roboquat merged commit 5c8f4c1 into main Nov 12, 2022

roboquat deleted the jp/user-service branch November 12, 2022 09:34

jeanp413 mentioned this pull request Nov 13, 2022

[public-api] IDEWorkspace service #14641

Merged

4 tasks

roboquat added deployed: webapp Meta team change is running in production deployed: IDE IDE change is running in production labels Nov 14, 2022

loujaybee added the component: public-api label Nov 21, 2022

roboquat added the deployed: workspace Workspace team change is running in production label Dec 7, 2022

[public-api] User service #14496

[public-api] User service #14496

Uh oh!

Conversation

jeanp413 commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release Notes

Documentation

Werft options:

Uh oh!

akosyakov commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

easyCZ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

easyCZ commented Nov 8, 2022

Uh oh!

akosyakov left a comment

Choose a reason for hiding this comment

Uh oh!

jeanp413 commented Nov 11, 2022

Uh oh!

werft-gitpod-dev-com bot commented Nov 11, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

easyCZ commented Nov 11, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akosyakov commented Nov 11, 2022

Uh oh!

easyCZ left a comment

Choose a reason for hiding this comment

Uh oh!

werft-gitpod-dev-com bot commented Nov 12, 2022

Uh oh!

jeanp413 commented Nov 12, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeanp413 commented Nov 12, 2022

Uh oh!

Uh oh!

jeanp413 commented Nov 8, 2022 •

edited

Loading

akosyakov commented Nov 8, 2022 •

edited

Loading

jeanp413 commented Nov 12, 2022 •

edited by werft-gitpod-dev-com bot

Loading