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
58 changes: 12 additions & 46 deletions docs/adr/0001-fake-broker-keeps-immediate-delivery.md
Original file line number Diff line number Diff line change
@@ -1,46 +1,12 @@
# The fake broker keeps immediate delivery; no in-memory `TimerStore`

**Decision:** `TestTimersBroker` keeps its current model — the fake producer encodes and dispatches
every published Timer immediately to the handler via `process_message`, regardless of its Activation
time. We do **not** add an `InMemoryTimerStore` adapter or a `TimerStore` `Protocol` for testing.

## Context

An architecture review proposed that "the fake fakes the wrong layer": put a real
`InMemoryTimerStore` behind the `TimerStore` seam so the fake broker exercises real Claim / Lease /
Commit and truthful inspection, instead of stubbing the Redis client and dispatching immediately.
Three defects were claimed: (1) the fake duplicates the envelope encode; (2) `has_pending`,
`get_pending_timers` and `cancel_all` are stubbed to canned empties ("lies"); (3) at-least-once and
Lease semantics cannot be tested through the fake. The options weighed were: A — drive the *real*
subscriber poll loop against an in-memory store; B — a real in-memory store for state with
deterministic delivery; or drop it.

## Decision & rationale

Researching FastStream's own Redis broker (`faststream/redis/testing.py`) dismantled the premises:

- **The encode is not duplicated.** FastStream's `FakeProducer.publish` also encodes via
`build_message` / `message_format.encode` — encoding is inherent to the fake-producer pattern, not
a smell. `FakeTimersProducer` mirrors it.
- **The inspection stubs are not lies.** FastStream fakes *every* subscriber — including the polling
list subscriber, our closest analog — by bypassing the poll loop and calling
`handler.process_message` directly from the fake producer; it never runs the real loop in tests.
Under an immediate-delivery contract a published Timer has already fired and been removed, so
`has_pending → False`, `get_pending_timers → []` and `cancel_all → 0` are *truthful*, not stubbed
lies.
- **Option A is non-idiomatic and fragile.** Running the real `_consume` loop under FastStream's
`TestBroker` means skipping `_fake_start` — forfeiting the handler-mock wiring every FastStream
test broker relies on — and driving an infinite 50 ms poll loop inside a unit test; a spike doing
so hung at teardown.

That leaves only one way to make inspection report *pending* future Timers: stop delivering future
Timers immediately. That is a **breaking change** to a public testing API — existing user tests
publish a future Timer and expect their handler to fire without waiting — so it is rejected. With
immediate delivery retained the inspection methods are already correct, and the proposal has no
remaining defect to fix. `scheduled_timers` already lets users assert what was Scheduled.

**Revisit trigger:** a concrete need to test Schedule / Pending / cancel semantics — a future Timer
observed as Pending before it fires — through the fake. If that arises, prefer an **opt-in**
`TestTimersBroker(..., respect_activation=True)` (default off, so non-breaking) that withholds
not-yet-Due Timers; not a change to the default immediate-delivery contract, and not a real
subscriber loop in tests.
# The fake broker delivers every Timer immediately

`FakeTimersProducer` encodes each published Timer and hands it straight to the matching subscriber's
`process_message`, ignoring the Activation time, and `_patch_broker` stubs the Redis client so
`has_pending` is False, `get_pending_timers` empty and `cancel_all` zero. Putting a real in-memory
store behind the `TimerStore` seam, so the fake exercised Claim, Lease and Commit, was rejected:
FastStream fakes every subscriber, its polling list subscriber included, by bypassing the poll loop,
and under immediate delivery a published Timer has already fired, so those inspection results are
truthful rather than canned. Driving the real `_consume` loop under `TestBroker` means skipping
`_fake_start` and running a 50 ms poll loop inside a unit test; a spike doing so hung at teardown.
Reporting a future Timer as Pending requires withholding it, which breaks every user test that
publishes one and expects its handler to fire, so it could only arrive as an opt-in flag.
40 changes: 0 additions & 40 deletions docs/adr/0002-keep-construction-factories.md

This file was deleted.

14 changes: 14 additions & 0 deletions docs/adr/0002-no-per-call-middlewares-kwarg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# No per-call `middlewares=` on subscribers, publishers and routes

`TimersRegistrator.subscriber()`, `.publisher()`, `TimersRoute` and `TimersRoutePublisher` take no
`middlewares=` keyword; `TimersBroker` and `TimersRouter` keep theirs, and broker or router scope is
the only way to wrap timer handling. FastStream 0.7 removed `add_call(middlewares_=...)`, so the
forwarding target no longer exists. Routing the kwarg to broker scope internally would have been a
silent change of meaning, since broker middleware runs for every Topic, and re-implementing
per-subscriber middleware locally would reproduce what upstream had just deleted while binding us to
an internal we had stopped depending on. The package was at version `"0"` with no stability promise,
so a hard break users see at import time was in policy. Behaving exactly like the brokers FastStream
ships is this integration's value, which is also why the single-caller `create_publisher` and
`create_subscriber` factories stay rather than being inlined into the registrator: they mirror
FastStream's own Redis broker layout. If upstream restores call-scoped middleware the kwarg returns
as a thin forward.
35 changes: 0 additions & 35 deletions docs/adr/0003-no-per-call-middlewares-kwarg.md

This file was deleted.

13 changes: 13 additions & 0 deletions docs/adr/0003-typing-extensions-over-version-gated-imports.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Backport 3.12-only typing via `typing_extensions`, not version-gated imports

Lowering the supported floor from 3.13 to 3.11 broke exactly two constructs: the PEP 695
`type RedisClient = ...` alias, a `SyntaxError` on 3.11, and `override` imported from `typing`,
which landed in 3.12. The alias became a plain `typing.TypeAlias` annotation, which needs no
backport; `override` is imported from `typing_extensions` unconditionally, never gated on
`sys.version_info`, and `typing-extensions` is a declared direct runtime dependency rather than a
pin borrowed from FastStream, which is free to drop it. A `sys.version_info` gate was rejected
because 3.11 needs the package installed either way, so it buys only more code at every affected
site and a second path for `ty` to check. Dropping `@override` was rejected because it is what
catches a signature drift against FastStream's base classes, precisely the failure this integration
is exposed to. When the floor rises to 3.12 both constructs gain stdlib spellings and the direct
dependency goes with them.
39 changes: 0 additions & 39 deletions docs/adr/0004-typing-extensions-over-version-gated-imports.md

This file was deleted.

5 changes: 3 additions & 2 deletions docs/agents/domain.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Single-context repo:
├── CONTEXT.md
├── docs/adr/
│ ├── 0001-fake-broker-keeps-immediate-delivery.md
│ └── 0002-keep-construction-factories.md
│ └── 0002-no-per-call-middlewares-kwarg.md
└── faststream_redis_timers/
```

Expand All @@ -32,4 +32,5 @@ If the concept you need isn't in the glossary yet, that's a signal: either you'r

If your output contradicts an existing ADR, surface it explicitly rather than silently overriding:

> _Contradicts ADR-0001 (fake broker keeps immediate delivery), but worth reopening because…_
> _Contradicts ADR-0001 (the fake broker delivers every Timer immediately), but worth reopening
> because…_
Loading