Skip to content

fix(redis): stop ListBatchPublisher from passing as a ListPublisher - #3242

Merged
IvanKirpichnikov merged 2 commits into
ag2ai:mainfrom
vyhuholl:fix/redis-list-batch-publisher-lsp
Sep 29, 2026
Merged

IvanKirpichnikov merged 2 commits into
ag2ai:mainfrom
vyhuholl:fix/redis-list-batch-publisher-lsp

Conversation

@vyhuholl

Copy link
Copy Markdown
Contributor

Description

Part of #3202.

ListBatchPublisher subclassed ListPublisher but replaced publish(message, list, reply_to, ...) with publish(*messages, list=..., ...). mypy accepts that override, since every positional argument still fits *messages, but the meaning changes. Code written against ListPublisher type checks a call that silently does something else on a batch publisher:

async def send(pub: ListPublisher) -> None:
    await pub.publish("hello", "other")  # "hello" to the list "other"


pub = broker.publisher(list=ListSub("default", batch=True))
await send(pub)
# on a ListBatchPublisher: ["hello", "other"] goes to "default" as one batch

It also applies without an explicit annotation: publisher(list=ListSub(q, batch=flag)) returns ListPublisher | ListBatchPublisher, and the same call type checks on that union. The snippet above behaves this way under TestRedisBroker.

Among pyrefly's 104 bad-override* findings on faststream/, this one (redis/publisher/usecase.py:263) is the only override a user can hit. The rest are summarized in #3202.

Fix

What the two publishers share (the constructor, list, subscriber_property, request) moves to _ListPublisherMixin, and ListPublisher and ListBatchPublisher both derive from it. The list subscribers are already split this way through _ListHandlerMixin, and Kafka's DefaultPublisher / BatchPublisher are siblings too. No method changes.

What changes for users: isinstance(batch_publisher, ListPublisher) is now False, and passing a ListBatchPublisher where ListPublisher is annotated is a type error. Nothing in faststream/, tests/, docs/docs_src/ or examples/ relies on either.

Tests

tests/mypy/redis.py passes a batch publisher to a function annotated with ListPublisher, under # type: ignore[arg-type]. On the old hierarchy mypy fails it with unused-ignore. Runtime behaviour isn't changed, so there's no runtime test.

Type of change

Please delete options that are not relevant.

  • 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
  • Both new and existing unit tests pass successfully on my local environment by running just test-coverage
  • I have ensured that static analysis tests are passing by running just static-analysis

ListBatchPublisher subclassed ListPublisher while replacing
`publish(message, list, ...)` with `publish(*messages, list=...)`. Code
typed against ListPublisher, including the `ListPublisher |
ListBatchPublisher` union that `publisher(list=ListSub(q, batch=flag))`
returns, type checked `pub.publish("hello", "other")`. On a batch
publisher that call sends ["hello", "other"] as one batch to the
publisher's own list.

The shared part (constructor, `list`, `subscriber_property`, `request`)
moves to `_ListPublisherMixin`, and both publishers derive from it, the
way `_ListHandlerMixin` already splits the list subscribers. No method
changes; `isinstance(batch_publisher, ListPublisher)` is now False.

pyrefly reported it as bad-override on `ListBatchPublisher.publish`, the
one override among its findings that a user can hit (ag2ai#3202).
@github-actions github-actions Bot added the Redis Issues related to `faststream.redis` module and Redis features label Sep 26, 2026
Moving `request()` into `_ListPublisherMixin` counts its lines as changed,
and diff-cover found no test that ever calls it: the shared request
testcase only builds a channel publisher for redis. The new case runs in
memory and against a real Redis.
Comment thread faststream/redis/publisher/usecase.py
Comment thread tests/brokers/redis/test_requests.py
Comment thread tests/mypy/redis.py
@IvanKirpichnikov
IvanKirpichnikov added this pull request to the merge queue Sep 29, 2026
Merged via the queue into ag2ai:main with commit a516d75 Sep 29, 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.

3 participants