diff --git a/docs/adr/0001-no-connection-provider-for-a-task.md b/docs/adr/0001-no-connection-provider-for-a-task.md index 7eac9c0..37d423b 100644 --- a/docs/adr/0001-no-connection-provider-for-a-task.md +++ b/docs/adr/0001-no-connection-provider-for-a-task.md @@ -1,26 +1,12 @@ # No connection provider for a task invocation -**Decision:** the per-task child container is built with no context, and this integration registers -no connection provider. - -`modern-di` defines a connection as the framework object a unit of work carries — the HTTP request -in the web integrations, the incoming message in the broker ones. Those adapters seed it as the -child container's context so a provider can depend on it, and the question asked of every new -integration is which object plays that role. It is asked here because Celery clearly *has* -per-invocation state: a task id, a retry count, message headers. - -That state lives on the `Task` instance, one layer above the seam. `inject` wraps the plain task -function, and `@app.task` sits outside `@inject`, so when the wrapper runs it holds the caller's -arguments and nothing else. Reaching the `Task` would mean either requiring `bind=True` on every -injected task or reading Celery's thread-local current task inside the wrapper — coupling this -package to task-instance internals to supply a context object that no provider it exists to serve -has needed. `modern-di-typer` declined the same thing for the same reason: a command, like a task, -is a plain call. - -So the unit of work is the task invocation itself: one `Scope.REQUEST` child per call, built with no -context, closed when the call returns or raises. A dependency that needs task metadata takes it as -an ordinary task parameter, passed by the caller. - -**Revisit trigger:** a provider that genuinely needs per-invocation Celery state rather than the -arguments the caller passed. At that point the context object has a concrete consumer, and the -coupling it costs is worth paying. +`modern-di` seeds a child container with the framework object a unit of work carries, the HTTP +request in the web integrations, the incoming message in the broker ones, so every integration must +name that object. Celery's per-invocation state, the task id, retry count and message headers, lives +on the `Task` instance, one layer above the seam: `inject` wraps the plain task function and +`@app.task` sits outside it, so the wrapper sees only the caller's arguments. Reaching the `Task` +would mean forcing `bind=True` on every injected task or reading Celery's thread-local current task, +coupling this package to task internals for a context object no provider needs; `modern-di-typer` +declined the same for a command. The unit of work is therefore the invocation: one `Scope.REQUEST` +child per call, built with no context, and a dependency needing task metadata takes it as an ordinary +task parameter. diff --git a/docs/adr/0002-no-functools-wraps-on-the-inject-wrapper.md b/docs/adr/0002-no-functools-wraps-on-the-inject-wrapper.md deleted file mode 100644 index 886081d..0000000 --- a/docs/adr/0002-no-functools-wraps-on-the-inject-wrapper.md +++ /dev/null @@ -1,24 +0,0 @@ -# `inject`'s wrapper does not use `functools.wraps` - -**Decision:** the wrapper copies `__name__`, `__qualname__`, `__doc__` and `__module__` by hand and -sets `__signature__` to the visible signature. It does not use `functools.wraps`. - -Those four attributes are a subset of what `functools.wraps` copies, so the hand-copying reads as an -oversight and invites a one-line "cleanup". It is not an oversight. - -`functools.wraps` also sets `__wrapped__`, and `inspect.signature` follows `__wrapped__` — which -would hand Celery's argument binding the *original* signature, `FromDI` parameters included, and -make every caller supply them. That the code would still work by luck is the trap: `inspect` -stops unwrapping at an object that carries its own `__signature__`, so the explicit assignment does -win today. The correctness would then rest on the interaction of two attributes set for opposite -purposes, three lines apart, with nothing naming the dependency between them. - -The signature rewrite is the entire mechanism by which a `FromDI` parameter disappears from a task's -public API. Nothing in the wrapper should point `inspect` back at the un-rewritten function, and the -sibling `modern-di` integrations that rewrite signatures avoid `wraps` for the same reason. - -**Revisit trigger:** something in the ecosystem needs `__wrapped__` on a task — a debugger, a -documentation generator, or Celery itself — and losing it costs more than the ambiguity gains. - -**Related:** [ADR-0003](0003-reject-variadics-alongside-fromdi.md), which depends on the visible -signature being the one Celery binds against. diff --git a/docs/adr/0002-reject-variadics-alongside-fromdi.md b/docs/adr/0002-reject-variadics-alongside-fromdi.md new file mode 100644 index 0000000..b7298a3 --- /dev/null +++ b/docs/adr/0002-reject-variadics-alongside-fromdi.md @@ -0,0 +1,13 @@ +# `inject` rejects `*args`/`**kwargs` alongside a `FromDI` parameter + +Decorating a function that declares a variadic parameter *and* at least one `FromDI` parameter +raises `TypeError` at decoration time; a function with no `FromDI` parameter is returned unchanged +and may use variadics freely. The wrapper binds the caller's arguments to the visible signature and +calls by name, `func(**bound.arguments, **resolved)`, which is what makes injection insensitive to +where a `FromDI` parameter sits. By-name calling cannot forward a variadic: `Signature.bind` stores +the payload under the literal parameter names, so a task called with three positional arguments +receives one tuple named `args`. Forwarding positionally would surrender order insensitivity for +every task, and special-casing the two names misroutes again once a task declares a real parameter +called `args`. For the same reason the wrapper sets `__name__`, `__qualname__`, `__doc__` and +`__module__` by hand rather than using `functools.wraps`, whose `__wrapped__` points +`inspect.signature` back at the un-rewritten signature. diff --git a/docs/adr/0003-connect-both-worker-signal-pairs.md b/docs/adr/0003-connect-both-worker-signal-pairs.md new file mode 100644 index 0000000..f4e8b0e --- /dev/null +++ b/docs/adr/0003-connect-both-worker-signal-pairs.md @@ -0,0 +1,13 @@ +# `setup_di` connects both worker signal pairs + +`setup_di` connects the same open/close closures to `worker_process_init`/`worker_process_shutdown` +*and* to `worker_init`/`worker_shutdown`, four connections for two operations, because neither pair +alone covers every pool. The process pair fires only under prefork and solo, once per forked child, +keeping cached resources and finalizers fork-safe; the worker pair fires once in the main process and +is the only pair the gevent, eventlet and threads pools send, since those never fork. Connecting only +the process pair is the bug 3.0.1 fixed: under `modern-di` 3.x's mandatory-open lifecycle the root +container stayed closed under the non-forking pools and every `@inject` task raised. The overlap is +harmless: `open()` is a no-op on an open container, `close_sync()` when nothing was cached. +`weak=False` belongs to the same decision: Celery holds receivers weakly by default and these +closures have no other strong reference, so a weak connection lets them be collected and the handlers +never run. diff --git a/docs/adr/0003-reject-variadics-alongside-fromdi.md b/docs/adr/0003-reject-variadics-alongside-fromdi.md deleted file mode 100644 index e35bf58..0000000 --- a/docs/adr/0003-reject-variadics-alongside-fromdi.md +++ /dev/null @@ -1,30 +0,0 @@ -# `inject` rejects `*args`/`**kwargs` alongside a `FromDI` parameter - -**Decision:** decorating a function that declares a `VAR_POSITIONAL` or `VAR_KEYWORD` parameter -*and* at least one `FromDI` parameter raises `TypeError` at decoration time. A task with no `FromDI` -parameter is returned unchanged and may use variadics freely. - -The wrapper binds the caller's arguments to the visible signature and calls the task by name -(`func(**bound.arguments, **resolved)`). Binding by name is what makes injection insensitive to -where a `FromDI` parameter sits in the parameter list — a property the suite tests, and the reason -`@inject` can be applied to a task whose dependencies are declared first, last, or interleaved. - -By-name calling cannot faithfully forward a variadic. `Signature.bind` stores the payload of `*args` -and `**kwargs` under the literal parameter names, so the re-expansion passes a tuple as a keyword -argument called `args` — a task called with three positional arguments receives one, named wrong. -Two alternatives were rejected: - -- **Forward positionally instead.** Gives up by-name binding, and with it order insensitivity, - for every task in order to serve the combination that provoked the bug. -- **Special-case the two names when re-expanding.** Works until a task declares an ordinary - parameter genuinely named `args` or `kwargs`, at which point it silently misroutes again — the - same failure, moved somewhere rarer and harder to find. - -Refusing the combination costs a signature nobody has asked for, and it fails at import time rather -than corrupting a payload on a worker. The error names the offending parameter and says what to -write instead. - -**Revisit trigger:** a real task needs both — a variadic fan-out signature that also wants a -resolved dependency. By-name binding would then have to be replaced by a call reconstruction that -preserves order insensitivity, and the two rejected options above are the list of what it must not -regress to. diff --git a/docs/adr/0004-connect-both-worker-signal-pairs.md b/docs/adr/0004-connect-both-worker-signal-pairs.md deleted file mode 100644 index 51aec14..0000000 --- a/docs/adr/0004-connect-both-worker-signal-pairs.md +++ /dev/null @@ -1,30 +0,0 @@ -# `setup_di` connects both worker signal pairs - -**Decision:** `setup_di` connects the same open/close closures to -`worker_process_init`/`worker_process_shutdown` *and* to `worker_init`/`worker_shutdown` — four -connections for two operations, all with `weak=False`. - -Four connections running two closures looks like duplication, and the obvious edit is to keep one -pair. Neither pair alone is sufficient, because they cover different pool families. - -`worker_process_init`/`worker_process_shutdown` fire only under the prefork and solo pools, once per -forked child under prefork. That per-child open/close is what keeps cached resources and finalizers -fork-safe: a forked process gets its own open container rather than inheriting one whose cached -values were created before the fork. `worker_init`/`worker_shutdown` fire once in the main worker -process for every pool, and are the *only* pair the gevent, eventlet and threads pools send, because -those pools run tasks in the main process and never fork. - -Shipping only the per-process pair is what 3.0.1 had to fix: under `modern-di` 3.x's mandatory-open -lifecycle the root container was never opened under the non-forking pools, and every `@inject` task -raised. Shipping only the worker pair would leave prefork children sharing state opened before the -fork. The overlap under prefork and solo is therefore deliberate and harmless: `Container.open()` is -a no-op on an already-open container, and `close_sync()` is a no-op when nothing was cached. - -`weak=False` is part of the same decision, not a style choice. Celery's signal dispatcher holds -receivers by weak reference by default, and these closures are local to `setup_di` with no other -strong reference to keep them alive — a weak connection lets them be collected before any worker -fires the signal, and the handlers then silently never run. The failure is invisible in eager tests -and total in a real worker. - -**Revisit trigger:** Celery changes which pools emit which of the four signals, or adds a pool family -that emits neither pair. diff --git a/docs/agents/domain.md b/docs/agents/domain.md index ff92e5c..145046a 100644 --- a/docs/agents/domain.md +++ b/docs/agents/domain.md @@ -18,7 +18,7 @@ Single-context repo: ├── CONTEXT.md ├── docs/adr/ │ ├── 0001-no-connection-provider-for-a-task.md -│ └── 0002-no-functools-wraps-on-the-inject-wrapper.md +│ └── 0002-reject-variadics-alongside-fromdi.md └── modern_di_celery/ ```