Closed Mark-Simulacrum closed 1 year ago
Seems like a pretty solid game plan :). I will certainly run into questions throughout implementation (esp. cuz I haven't worked with the DB on the rust front a whole lot)
If you were to theorize about how you'd want to split this up into PRs, what would you be thinking. If it was TMF, I'd imagine it'd look something like this...
lmk if i'm on the right track or if you've got a different idea for how this should look :)
I would bundle 1+2 in your list into a single PR, so that we can prove out that the database layer makes sense when actually trying to use it in code (not always actually true). I'd estimate it shouldn't be that much code-wise -- maybe ~500 lines at most -- so hopefully not too large of a PR, though obviously if it has to be more that's fine.
Sounds good. I will start working on this on Monday :)
@rustbot claim
I believe this is somewhat distinct from https://github.com/rust-lang/triagebot/issues/1595, which is more about regularly scheduled events (e.g., every Monday at 1pm ET) -- supporting cron jobs is likely somewhat easier than scheduled events, since we can hardcode checking into triagebot and avoid a database entirely most likely. (Modulo triagebot rebooting at inopportune times). It may be that we actually end up sharing the scheduled events code, with cron jobs queueing an event to happen in X time at first startup if such an event doesn't exist yet.
For scheduled events, I believe our design should look something like this:
New postgres table for the events, with these columns (roughly):
We'll have a background thread that polls the database and every minute (maybe more often, but we can start with every minute):
Alongside this there's probably going to be value in a few helper methods that add events to the table and remove them.
For cron support atop this framework, we'd likely do that by running at startup a little bit of code that checks if there are any events queued for cron-task-$name, and if not, then queue up an event with the appropriate delta from now. We'd then re-queue cron-task events when handling them at the appropriate time. But this can be a separate part of the work.
cc @chazkiker2