Skip to content

chore: watch workspace endpoint #4060

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 34 commits into from
Sep 16, 2022
Merged

Conversation

f0ssel
Copy link
Contributor

@f0ssel f0ssel commented Sep 14, 2022

Example output:

coder git:(f0ssel/workspace-watch-with-resources) ✗ curl -N -v "http://localhost:3000/api/v2/workspaces/3a466773-3444-4e0a-9844-09ab7e5928f5/watch?coder_session_token=rBwG7Hr7Ea-WjbsYAAkPgY1EnK19372Ey" 
*   Trying 127.0.0.1:3000...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET /api/v2/workspaces/3a466773-3444-4e0a-9844-09ab7e5928f5/watch?coder_session_token=rBwG7Hr7Ea-WjbsYAAkPgY1EnK19372Ey HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Build-Version: v0.8.14-devel+bee6320e
< Cache-Control: no-cache
< Connection: keep-alive
< Content-Type: text/event-stream
< Set-Cookie: csrf_token=Silr1gFgOkXAFzOsB0a9QXXenZR78nf6QVLEBFtbMQQ=; Path=/; HttpOnly; SameSite=Lax
< Vary: Cookie
< X-Accel-Buffering: no
< X-Coder-Request-Id: 30c08697-6c2e-40f7-a71d-892dcd780346
< X-Ratelimit-Limit: 512
< X-Ratelimit-Remaining: 512
< X-Ratelimit-Reset: 1663255020
< Date: Thu, 15 Sep 2022 15:16:41 GMT
< Transfer-Encoding: chunked
< 
event: ping

event: data
data: {"id":"3a466773-3444-4e0a-9844-09ab7e5928f5","created_at":"2022-09-14T17:56:04.401166Z","updated_at":"2022-09-14T17:56:04.401166Z","owner_id":"86790dba-0379-4b3c-9f88-5a35c230667e","owner_name":"admin","template_id":"3b7c637b-cfe6-4a4e-991b-ac4bf35a2e76","template_name":"docker","template_icon":"","latest_build":{"id":"c31e01fe-a6db-4c8b-949e-cecb8b2c3c6a","created_at":"2022-09-14T17:56:04.401166Z","updated_at":"2022-09-14T17:56:25.446936Z","workspace_id":"3a466773-3444-4e0a-9844-09ab7e5928f5","workspace_name":"wascasc","workspace_owner_id":"86790dba-0379-4b3c-9f88-5a35c230667e","workspace_owner_name":"admin","template_version_id":"f7f91f81-35fb-41af-bdb3-b34ed3deda47","build_number":1,"transition":"start","initiator_id":"86790dba-0379-4b3c-9f88-5a35c230667e","initiator_name":"admin","job":{"id":"405aa0f4-8f35-4a8b-8dc7-e666d887319c","created_at":"2022-09-14T17:56:04.401166Z","started_at":"2022-09-14T17:56:04.59518Z","completed_at":"2022-09-14T17:56:25.447357Z","status":"succeeded","worker_id":"ccf6e422-dd11-4eb4-a7f8-c51616ea45ad","storage_source":"f0075e9c8efa422652abfbaf4792a6b35ec9a3dc103af00653b1716a66c18555"},"reason":"initiator","deadline":null,"resources":[{"id":"9b48e3dd-f1ed-4160-9386-93d218a1c2a0","created_at":"2022-09-14T17:56:25.449063Z","job_id":"405aa0f4-8f35-4a8b-8dc7-e666d887319c","workspace_transition":"start","type":"docker_container","name":"workspace","hide":false,"icon":"","agents":[{"id":"3b08cd19-bb03-4bc3-8d43-a63d32609a60","created_at":"2022-09-14T17:56:25.449565Z","updated_at":"2022-09-15T15:16:40.896075Z","first_connected_at":"2022-09-14T17:56:28.876449Z","last_connected_at":"2022-09-15T15:16:40.896074Z","status":"connected","name":"main","resource_id":"9b48e3dd-f1ed-4160-9386-93d218a1c2a0","architecture":"amd64","environment_variables":{"GIT_AUTHOR_EMAIL":"admin@coder.com","GIT_AUTHOR_NAME":"admin","GIT_COMMITTER_EMAIL":"admin@coder.com","GIT_COMMITTER_NAME":"admin"},"operating_system":"linux","startup_script":"code-server --auth none","version":"v0.8.14-devel+cc472728","apps":[{"id":"ad577184-93e7-4559-b150-38c13701b19a","name":"code-server","icon":"/icon/code.svg"}],"latency":{"Coder Embedded Relay":{"preferred":true,"latency_ms":21.443975000000002}}}],"metadata":[{"key":"type","value":"docker_container","sensitive":false}]},{"id":"5cf04e2b-ba95-4789-ac75-e7887977fdde","created_at":"2022-09-14T17:56:25.450508Z","job_id":"405aa0f4-8f35-4a8b-8dc7-e666d887319c","workspace_transition":"start","type":"docker_volume","name":"home_volume","hide":false,"icon":"","metadata":[{"key":"type","value":"docker_volume","sensitive":false}]}]},"outdated":false,"name":"wascasc","last_used_at":"0001-01-01T00:00:00Z"}

What's changed

  • New watch endpoint that uses SSE to stream the state of the workspace to the client on an interval.
    • This will be replaced with real time updates in a future PR.
  • We now return workspace resources everywhere we return codersdk.Workspace

@f0ssel f0ssel requested a review from deansheather September 14, 2022 21:23
@f0ssel f0ssel marked this pull request as ready for review September 14, 2022 21:26
@f0ssel f0ssel requested a review from a team as a code owner September 14, 2022 21:26
@f0ssel f0ssel requested review from BrunoQuaresma and Kira-Pilot and removed request for a team September 14, 2022 21:26
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

The FE looks good but my concern is to start to use different approaches for "real-time" data. We are using on workspace web sockets and here server events.

Copy link
Member

@Kira-Pilot Kira-Pilot left a comment

Choose a reason for hiding this comment

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

looks like it's working to me!

Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

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

Cool but it's server-sent events not server-side events

@f0ssel f0ssel requested a review from deansheather September 16, 2022 17:12
Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

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

It's a lot of code but at least it's not doing queries in a loop

@f0ssel f0ssel force-pushed the f0ssel/workspace-watch-with-resources branch from ace0256 to 4e95a99 Compare September 16, 2022 18:46
@f0ssel f0ssel enabled auto-merge (squash) September 16, 2022 18:53
@f0ssel f0ssel merged commit 63fd494 into main Sep 16, 2022
@f0ssel f0ssel deleted the f0ssel/workspace-watch-with-resources branch September 16, 2022 18:54
Comment on lines +12 to +25
type ServerSentEvent struct {
Type ServerSentEventType `json:"type"`
Data interface{} `json:"data"`
}

type ServerSentEventType string

const (
ServerSentEventTypePing ServerSentEventType = "ping"
ServerSentEventTypeData ServerSentEventType = "data"
ServerSentEventTypeError ServerSentEventType = "error"
)

func ServerSentEventReader(rc io.ReadCloser) func() (*ServerSentEvent, error) {
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, none of these need to be exposed from the codersdk. I see ping is used as a const, but that seems entirely reasonable to just hardcode in both places since it's tested code anyways.

I'd prefer to keep as much private as we can in our SDK, because we want to limit breaking changes in the future to API consumers. This can obvs be fixed in a future PR!

Comment on lines +12 to +25
type ServerSentEvent struct {
Type ServerSentEventType `json:"type"`
Data interface{} `json:"data"`
}

type ServerSentEventType string

const (
ServerSentEventTypePing ServerSentEventType = "ping"
ServerSentEventTypeData ServerSentEventType = "data"
ServerSentEventTypeError ServerSentEventType = "error"
)

func ServerSentEventReader(rc io.ReadCloser) func() (*ServerSentEvent, error) {
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, none of these need to be exposed from the codersdk. I see ping is used as a const, but that seems entirely reasonable to just hardcode in both places since it's tested code anyways.

I'd prefer to keep as much private as we can in our SDK, because we want to limit breaking changes in the future to API consumers. This can obvs be fixed in a future PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants