diff --git a/docs/adr/0001-per-task-scope-rides-taskiq-dependencies.md b/docs/adr/0001-per-task-scope-rides-taskiq-dependencies.md index 3e17f2b..9d70448 100644 --- a/docs/adr/0001-per-task-scope-rides-taskiq-dependencies.md +++ b/docs/adr/0001-per-task-scope-rides-taskiq-dependencies.md @@ -1,29 +1,12 @@ # Per-task scope rides a taskiq generator dependency, not a middleware -**Decision:** the per-task child is built by a generator `TaskiqDepends` (`build_di_container`); we -will not ship a `TaskiqMiddleware` that opens and closes a container around every task. - -A middleware is the obvious place to hang a per-unit-of-work scope, and it is what Dishka's taskiq -integration does, so the option comes back on its own. It was rejected because taskiq already -provides the exact contract: a generator dependency is resolved **once per task** under the default -`use_cache=True`, its yielded value is shared by every dependent in that task, and it is finalized -after the task completes — including when the task raises, because taskiq throws the task's -exception into the generator at the `yield`. That is child-container-per-unit-of-work, already -built. - -Taking it as a dependency rather than a middleware buys three things a middleware cannot. It is -**lazy**: a task with no `FromDI` parameter resolves no dependency and therefore builds no child, so -a broker with one wired task pays nothing on the others. It needs **no registry**: the child reaches -the parameters through taskiq's own dependency cache, so there is nothing keyed by task id to -populate, look up, and clean up, and nothing to leak if a task dies between the two halves of a -middleware. And it requires **no installation step beyond `setup_di`** — a middleware would have to -be registered on the broker as well, giving a second way to get the wiring half-done. - -The middleware also fails the deletion test in reverse: adding one would not remove the generator -dependency, because `FromDI` parameters still need a container handed to them at resolve time. It -would be a second mechanism layered over the one that already works. - -**Revisit trigger:** taskiq changes the caching or finalization semantics of generator dependencies -— a yielded value no longer shared across a task's parameters, or a finalizer no longer run on the -error path — or a required feature genuinely needs to act before the first `FromDI` parameter is -resolved (per-task container setup that must happen even for tasks that inject nothing). +The per-task child container is built by `build_di_container`, an async generator `TaskiqDepends`; +no `TaskiqMiddleware` is shipped. A middleware is the obvious place to hang a per-unit-of-work +scope and is what Dishka's taskiq integration does, so the option keeps resurfacing, but taskiq +already provides the contract: under the default `use_cache=True` a generator dependency resolves +once per task, its yielded value is shared by every dependent, and taskiq throws the task's +exception into it at the `yield`, so the child closes on the error path too. Taking it as a +dependency is also lazy, since a task with no `FromDI` parameter builds no child; it needs no +task-id-keyed registry to leak; and it needs no installation step beyond `setup_di`. A middleware +would only add a second mechanism, because `FromDI` parameters still need a container handed to +them at resolve time. diff --git a/docs/adr/0002-only-worker-lifecycle-is-wired.md b/docs/adr/0002-only-worker-lifecycle-is-wired.md index 3770e6a..81ef93f 100644 --- a/docs/adr/0002-only-worker-lifecycle-is-wired.md +++ b/docs/adr/0002-only-worker-lifecycle-is-wired.md @@ -1,29 +1,12 @@ # `setup_di` wires only the worker lifecycle events -**Decision:** `setup_di` registers handlers for `WORKER_STARTUP` and `WORKER_SHUTDOWN` only; the -`CLIENT_STARTUP` / `CLIENT_SHUTDOWN` pair is deliberately left unwired. - -taskiq fires two independent lifecycle pairs, and wiring both looks like the safe default. It was -rejected because the worker is the only side that resolves. A kicker process constructs the broker -and calls `.kiq()`; it never runs a task, so a container opened on `CLIENT_STARTUP` would hold app -scoped resources — connections, pools, whatever the providers create at open — for a process that -resolves nothing from them, and would have to close them again on a shutdown event that a -short-lived client script frequently never fires. Wiring the pair that matches where resolution -happens keeps the container's lifetime equal to the span in which it is used. - -The cost is a real one, so it is stated rather than hidden: a process that both kicks and executes -in-process, which is what `InMemoryBroker` does in a test or a script, gets no lifecycle from -`setup_di` unless the worker events actually fire. `InMemoryBroker.startup()` fires both pairs, so -the in-process case works; a caller driving tasks by other means opens and closes the root container -itself. This is documented in `README.md`, because it is the one place the choice is visible to a -user. - -`container.open()` on `WORKER_STARTUP` is unconditional for the same reason. A fresh `Container` is -already open, so the first call is a no-op; the call earns its keep on the **second** worker cycle — -a restart, or a test that starts and stops the same broker twice — where the container was closed by -the previous `WORKER_SHUTDOWN` and resolving without reopening would raise `ContainerClosedError`. - -**Revisit trigger:** a client-side capability appears that resolves from the container before any -task runs — for instance a kicker-side provider used to build task arguments, or middleware on the -client path that needs DI. At that point the client is a resolving context and needs its own -lifecycle. +`setup_di` registers handlers for `WORKER_STARTUP` and `WORKER_SHUTDOWN` only and leaves +`CLIENT_STARTUP` / `CLIENT_SHUTDOWN` unwired, because the worker is the only side that resolves. A +kicker constructs the broker and calls `.kiq()` without ever running a task, so a container opened +on `CLIENT_STARTUP` would hold app-scoped connections and pools for a process that resolves nothing +from them, then close them on a shutdown event a short-lived script often never fires. The cost is +documented in `README.md` rather than hidden: a process that both kicks and executes gets no +lifecycle unless the worker events fire, which `InMemoryBroker.startup()` arranges by firing both +pairs. `container.open()` on worker startup is unconditional and a no-op on a fresh container; it +earns its keep on a second worker cycle, reopening deliberately what the previous `WORKER_SHUTDOWN` +closed instead of leaving modern-di to reopen it implicitly with a `ContainerClosedWarning`.