Skip to content

Require a destination in subscriber() and publisher() overloads - #3234

Merged
IvanKirpichnikov merged 3 commits into
ag2ai:mainfrom
vyhuholl:fix/redis-overload-destination
Sep 30, 2026
Merged

IvanKirpichnikov merged 3 commits into
ag2ai:mainfrom
vyhuholl:fix/redis-overload-destination

Conversation

@vyhuholl

Copy link
Copy Markdown
Contributor

Description

Part of #3202.

Every specific overload of RedisRegistrator.subscriber() / .publisher() and of the FastAPI RedisRouter gave its destination a ... default. A call that names no destination matched the first overload:

broker = RedisBroker()

broker.subscriber()  # typed as ChannelSubscriber
broker.publisher()   # typed as StreamPublisher
# both raise at runtime:
# faststream.exceptions.SetupError: You have to specify `channel`, `list` or `stream`

pyrefly reports this as inconsistent-overload-default at redis/broker/registrator.py:49, 371 and redis/fastapi/fastapi.py:391, 724: the implementation defaults the destination to None, and the overload types don't admit it. The overloads lied about what may be omitted. The None default itself is live, since list= and stream= are passed without a channel.

Fix

  • The destination an overload dispatches on is required there: channel in the channel overloads, list and stream in theirs.
  • RedisRegistrator's catch-all overload for destinations known only at runtime (str | None) is split in three, one per destination. subscriber() and publisher() with no destination now match no overload, while broker.subscriber(stream=maybe_name) still resolves to LogicSubscriber. The FastAPI router never had a catch-all and doesn't get one.
  • In the concurrent overloads max_workers: int = ... becomes max_workers: int. Once the destination was required, mypy reported the concurrent list and stream overloads as overlapping the plain ones (overload-overlap), because they too claimed max_workers could be left out.
  • The FastAPI router's publisher() takes list and stream positionally, and a required parameter can't follow a defaulted positional one. Its list and stream overloads make them keyword-only, as RedisRegistrator.publisher() already does. Its first overload now carries # type: ignore[override], like the Kafka router's: the abstract StreamRouter.publisher(self) takes no arguments, and no overload accepts that call any more. pyrefly counts this as one new bad-override, the same finding the Kafka and Confluent routers already have.

No runtime change. The type checker now rejects calls with no destination, which always raised, and positional list / stream on the FastAPI router's publisher (router.publisher(None, "name")), which still works at runtime.

The other brokers keep their batch: Literal[True] = ..., max_workers: int = ... and no_confirm: Literal[True] = ... defaults. An earlier overload catches the call that omits them, so there the default never selects the overload.

Tests

In tests/mypy/redis.py:

  • check_destination_is_required calls subscriber() and publisher() with no destination on RedisBroker, RedisRouter and the FastAPI RedisRouter, under # type: ignore[call-overload]. With faststream/ reverted, mypy fails it with two unused-ignore errors.
  • check_runtime_destination_type pins that a str | None channel, list or stream still resolves to LogicSubscriber / LogicPublisher, so splitting the catch-all dropped nothing.

Checked

  • just mypy (1883 files), just pyright and just pyrefly are clean, just linter passes.
  • pyrefly check faststream: the four inconsistent-overload-default findings are gone; the bad-override above is the only new one.
  • tests/brokers/redis/test_misconfigure.py, test_fastapi.py, test_router.py and test_option_surface.py pass in memory (65 tests).

Type of change

  • Bug fix (a non-breaking change that resolves an issue)

Checklist

  • My code adheres to the style guidelines of this project (just lint shows no errors)
  • I have conducted a self-review of my own code
  • My changes do not generate any new warnings
  • I have added tests to validate the effectiveness of my fix or the functionality of my new feature
  • just mypy, just pyright and just pyrefly pass

…rloads

Every specific overload gave its destination a `...` default, so a call
with no channel, list or stream matched the first one:
`broker.subscriber()` type checked as `ChannelSubscriber` and
`broker.publisher()` as `StreamPublisher`, while both raise `SetupError`
at runtime. pyrefly reports it as inconsistent-overload-default (ag2ai#3202).

The destination an overload dispatches on is now required. The catch-all
for destinations known only at runtime is split in three, one per
destination, so a call naming none matches no overload. Once the
destination is required, the concurrent overloads' `max_workers: int = ...`
overlaps the plain ones, so `max_workers` is required there too.

The FastAPI router's publisher takes `list` and `stream` positionally; a
required one cannot follow a defaulted positional, so those overloads make
them keyword-only, as RedisRegistrator.publisher already does.
@github-actions github-actions Bot added the Redis Issues related to `faststream.redis` module and Redis features label Sep 24, 2026
Comment thread tests/mypy/redis.py Outdated
…ode covers

With a union receiver the no-destination case stayed green unless both
RedisRegistrator and the FastAPI router regressed: mypy reports the call
as long as one member rejects it. Each implementation gets its own line
now; RedisRouter shares RedisRegistrator with RedisBroker.

The channel-positional runtime case goes: `RedisClusterBroker.subscriber`
and the FastAPI router's `publisher` already forward `str | None`
destinations that way, so `just mypy` fails without that overload. The
list- and stream-only forms have no caller in the library and stay.
@IvanKirpichnikov
IvanKirpichnikov added this pull request to the merge queue Sep 29, 2026
Merged via the queue into ag2ai:main with commit c8e928c Sep 30, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Redis Issues related to `faststream.redis` module and Redis features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants