Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 10 additions & 27 deletions docs/adr/0001-per-task-scope-rides-taskiq-dependencies.md
Original file line number Diff line number Diff line change
@@ -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.
37 changes: 10 additions & 27 deletions docs/adr/0002-only-worker-lifecycle-is-wired.md
Original file line number Diff line number Diff line change
@@ -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`.
Loading