let TaskRunRecorder process events into task runs/task run states
#14729
Conversation
| } | ||
| session = await stack.enter_async_context(db.session_context()) | ||
|
|
||
| await _insert_task_run(session, task_run, task_run_attributes) |
There was a problem hiding this comment.
Unfortunately due to FK's we need to insert the task run, then insert the task run state, then go back and update the task run with the denormalized state attributes
There was a problem hiding this comment.
Okay, sometimes this trips me up with the db.session_context(): begin_transaction defaults to False, so these aren't in a transaction, right? It feels like they should be
There was a problem hiding this comment.
db.session_context(..., with_for_update=True) as well here?
There was a problem hiding this comment.
Good question, that might make the lock a little more aggressive? I'm thinking about the SQLite implementation hitting database is locked. Since this should be the only thing writing task runs, I think we're good with just the transaction
| db.TaskRun.id == task_run.id, | ||
| sa.or_( | ||
| db.TaskRun.state_timestamp.is_(None), | ||
| db.TaskRun.state_timestamp < task_run.state.timestamp, |
There was a problem hiding this comment.
I think this fits our pattern so that we'll only do this if things are in order
CodSpeed Performance ReportMerging #14729 will not alter performanceComparing Summary
|
| } | ||
| session = await stack.enter_async_context(db.session_context()) | ||
|
|
||
| await _insert_task_run(session, task_run, task_run_attributes) |
There was a problem hiding this comment.
Okay, sometimes this trips me up with the db.session_context(): begin_transaction defaults to False, so these aren't in a transaction, right? It feels like they should be
closes: #14648
Updates
TaskRunRecorderfrom being a no-op service to actually processingtask-runevents that come through.For every event we see we'll: