-
Notifications
You must be signed in to change notification settings - Fork 936
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
Conversation
There was a problem hiding this 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.
There was a problem hiding this 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!
There was a problem hiding this 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
There was a problem hiding this 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
ace0256
to
4e95a99
Compare
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) { |
There was a problem hiding this comment.
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!
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) { |
There was a problem hiding this comment.
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!
Example output:
What's changed