Unified attempts list on Job detail #346
Conversation
| ByKind bool `json:"by_kind"` | ||
| } | ||
|
|
||
| type RiverJobMinimal struct { |
There was a problem hiding this comment.
Not sure if you have another preferred pattern for this, but with the prospect for larger amounts of data being stored in metadata it doesn't seem like a great idea to fetch or render the entire thing into the API response. This was just a quick way to get unblocked and put the problem forward.
Actually the query itself is untouched here since we're just leveraging JobList, so it's fetching the payloads whether or not they're used. But still, the frontend doesn't need to keep these for a potentially long list of jobs being displayed, and then re-fetching it every few seconds.
There was a problem hiding this comment.
It is indeed the idiomatic way to segregate "exported" data to the dedicated struct.
There was a problem hiding this comment.
We use a similar approach at work, even using the "minimal" moniker, but what do you think about embedding the minimal job on the main job like?
type RiverJob struct {
RiverJobMinimal
}The JSON fields end up at the same top level, but that way we don't need to maintain two completely duplicate lists of values, and it's obvious what's different in the minimal version versus non-minimal version.
There was a problem hiding this comment.
I couldn't remember whether it came out unnested or not in the JSON, so great news, I've made that switch.
| ByKind bool `json:"by_kind"` | ||
| } | ||
|
|
||
| type RiverJobMinimal struct { |
There was a problem hiding this comment.
It is indeed the idiomatic way to segregate "exported" data to the dedicated struct.
brandur
left a comment
There was a problem hiding this comment.
Yes! Looking great.
This includes which worker ran the job, errors if they occurred, and any logs which may have been recorded with the new riverlog middleware.
Nice! Okay, I didn't check this because I don't have a great way locally of producing any test data, but in case the job does not error, does this approach give the user a way to see the logs for a non-error run?
Side note: while building this, I realized this is undocumented in riverqueue.com/docs—we may want to fix that?
Yeah, I was basically going to wait until there was UI support, but after this ships let's put them in for sure.
| ByKind bool `json:"by_kind"` | ||
| } | ||
|
|
||
| type RiverJobMinimal struct { |
There was a problem hiding this comment.
We use a similar approach at work, even using the "minimal" moniker, but what do you think about embedding the minimal job on the main job like?
type RiverJob struct {
RiverJobMinimal
}The JSON fields end up at the same top level, but that way we don't need to maintain two completely duplicate lists of values, and it's obvious what's different in the minimal version versus non-minimal version.
This is a partial redesign of the job detail page to attempt to bring all attempt-related info into a single attempts list. This includes which worker ran the job, errors if they occurred, and any logs which may have been recorded with the new
riverlogmiddleware. Side note: while building this, I realized this is undocumented in riverqueue.com/docs—we may want to fix that?Currently based on #344 because the conflicts would have been annoying.