refactor(run-store,webapp,run-engine): route Postgres TaskRun reads through the run store#3990
refactor(run-store,webapp,run-engine): route Postgres TaskRun reads through the run store#3990d-cs wants to merge 11 commits into
Conversation
Add findRun, findRunOrThrow and findRuns to RunStore, mirroring the existing write methods. They pass where/select/include through the same Prisma generics and default to the read replica, while letting the caller pass the writer or a transaction client when needed. This lets Postgres reads of TaskRun be routed through the store the same way writes already are. Additive only; no call sites change yet.
Add a no-args overload to findRun, findRunOrThrow and findRuns that returns the whole TaskRun row, for callers that read a run without a select or include.
Relocate the direct TaskRun reads in the engine and its systems to the RunStore read methods, preserving the exact client (writer, replica, or transaction) at each site. Behavior-preserving; the engine test suite is unchanged.
…tore Relocate the direct TaskRun reads in webapp services, run-engine concerns, realtime, mollifier and metadata to the RunStore read methods, preserving the exact client (writer, replica, or transaction) at each site. The run hydrator now receives the store by injection. Behavior-preserving.
Relocate the dashboard presenter TaskRun reads to the RunStore read methods, preserving the exact client per site. Behavior-preserving.
…store Relocate the route and loader TaskRun reads to the RunStore read methods, preserving the exact client per site, including the replica-resolve then writer-recheck realtime paths. Behavior-preserving.
…store Decompose the three reads that pulled TaskRun in through a parent model's relation include (alert, batch results, attempt dependencies): query the parent without the include, hydrate the run(s) via RunStore in a single batched read, and stitch them back. Preserves field selection, ordering, null handling and the query client. Adds container-backed tests for the batch-results and cancel-dependencies paths.
…tover The recovery script joins TaskRunExecutionSnapshot to TaskRun in raw SQL, so it is the one TaskRun read not routed through the run store. Add a note to revisit it at table cutover.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The new container tests import the service and presenter, which pull the db.server singleton in through their base classes. Mock it so the tests do not try to connect to the env database when none is reachable (the CI unit shards), matching the existing webapp container-test pattern. The tests use the injected testcontainer prisma for all reads.
Importing the service pulls the cancel chain, which eagerly initializes the concurrency tracker singleton and requires REDIS_HOST/REDIS_PORT at import time, so the suite cannot load in the unit-test shards without stacking mocks. The decompose it covered is exercised by the analogous batch-results container test and confirmed by review, so drop this one rather than mock the tracker and cancel chain.
Summary
Adds read methods to
RunStore(findRun,findRunOrThrow,findRuns) and routes every Postgres read ofTaskRunthrough them, mirroring how writes already go through the store. Behavior-preserving: each relocated read keeps its exact query, field selection, and database client (writer, replica, or transaction). This letsTaskRunreads be retargeted to a different backing store later without touching call sites.Stacked on #3981 (the write adapter); that PR is the base of this one.
Scope
In scope: the run engine, webapp services, presenters, and route loaders. Three reads that pulled
TaskRunin through a parent model's relationinclude(alert delivery, batch results, attempt-dependency cancellation) are decomposed to fetch the run(s) through the store and stitch them back, since a relation include would not followTaskRunto a new table.Left reading the existing table (out of scope): the legacy MarQS paths, the legacy trigger idempotency read, and one raw-SQL recovery script (commented for revisiting at cutover).
Notes
Reads default to the read replica; callers pass the writer or a transaction client wherever the original read did, so writer-vs-replica behavior is unchanged.