feat: Add streaming endpoint for workspace history #157
Conversation
Enables a buildlog-like flow for reading job output.
Codecov Report
@@ Coverage Diff @@
## main #157 +/- ##
==========================================
- Coverage 66.55% 66.02% -0.54%
==========================================
Files 104 105 +1
Lines 5221 5501 +280
Branches 68 68
==========================================
+ Hits 3475 3632 +157
- Misses 1423 1524 +101
- Partials 323 345 +22
Continue to review full report at Codecov.
|
kylecarbs
force-pushed
the
jobstreaming
branch
2 times, most recently
from
0da0d10
to
d5eafa5
Compare
kylecarbs
force-pushed
the
jobstreaming
branch
from
d5eafa5
to
f825a60
Compare
kylecarbs
force-pushed
the
jobstreaming
branch
from
cb6c437
to
4421b46
Compare
| @@ -96,6 +97,7 @@ func New(options *Options) http.Handler { | |||
| r.Route("/{workspacehistory}", func(r chi.Router) { | |||
| r.Use(httpmw.ExtractWorkspaceHistoryParam(options.Database)) | |||
| r.Get("/", api.workspaceHistoryByName) | |||
| r.Get("/logs", api.workspaceHistoryLogsByName) | |||
| // The Go stdlib JSON encoder appends a newline character after message write. | ||
| encoder := json.NewEncoder(rw) |
There was a problem hiding this comment.
Thanks for adding the comment here, and above, for the ndjson spec
There was a problem hiding this comment.
lol is ndjson another format similar to https://jsonlines.org/ ??
They even forked it from that one, which seems functionally equivalent... why?!
There was a problem hiding this comment.
lmao no idea, I thought ndlines was the only one
There was a problem hiding this comment.
ah the answer was on the jsonlines site:
| logChan, err := server.Client.FollowWorkspaceHistoryLogsAfter(context.Background(), "", workspace.Name, workspaceHistory.Name, now) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
In addition to the follow case - it would be nice to have a few other cases:
before+follow-> should return errorafter+follow-> should stream logs, but just after the current timebefore+after, no follow -> should return a subset of logs, unstreamed- no args -> should return all the logs, unstreamed
There was a problem hiding this comment.
(non-blocking, just thinking about it).
This could actually be a nice good-first-issue for someone we're bringing on to help with workspaces v2, to get them familiar with the model and the streaming strategy we're using - adding tests is a great way to get to know the codebase.
There was a problem hiding this comment.
It's a bit strange to test before + follow, because the codersdk shouldn't support it, but we should technically test it. I'll noodle on how we should test those cases!
|
✅ for the test run on first try, happy times 😎 |
kylecarbs
force-pushed
the
jobstreaming
branch
from
c991528
to
8904dc5
Compare
Enables a buildlog-like flow for reading job output.