Repository navigation
feat(coderd): add experimental tasks logs endpoint - #19958
Conversation
472270a to
ff2b3a2
Compare
9e17127 to
b7830b0
Compare
mafredri
changed the title
562d075 to
daec105
Compare
DanielleMaywood
left a comment
DanielleMaywood
left a comment
There was a problem hiding this comment.
just a small nit on the httperror.WriteResponseError
Co-authored-by: Danielle Maywood <[email protected]>
johnstcn
left a comment
johnstcn
left a comment
There was a problem hiding this comment.
LGTM, some comments below but I wouldn't consider them blocking in an experimental endpoint.
| body, _ := io.ReadAll(io.LimitReader(resp.Body, 128)) | ||
| return httperror.NewResponseError(http.StatusBadGateway, codersdk.Response{ | ||
| Message: "Task app rejected the request.", | ||
| Detail: fmt.Sprintf("Upstream status: %d; Body: %s", resp.StatusCode, body), |
There was a problem hiding this comment.
👍 this should give a good indication to the user
| case "user": | ||
| typ = codersdk.TaskLogTypeInput | ||
| case "agent": | ||
| typ = codersdk.TaskLogTypeOutput |
There was a problem hiding this comment.
Is there a reason to have these be different? Could we instead use codersdk.TaskLogTypeUser / codersdk.TaskLogTypeAgent?
There was a problem hiding this comment.
I'm somewhat indifferent and open to changing this, this is just the last option we discussed/settled on with @DanielleMaywood (correct me if I misremember). It seemed like a good idea to try to tie the concept of send input into the logs (input/output) to make the origin clear.
There was a problem hiding this comment.
Let's go with this for now. We can adjust if need be.
There was a problem hiding this comment.
Yeah that was pretty much the reason, I'm also indifferent on what we pick here as well.
91730fe to
e2db31a
Compare
Both
followandtailhave been omitted for now.Fixes coder/internal#901