Skip to content

fix(redis): honor no_ack in manual stream reads - #3274

Merged
IvanKirpichnikov merged 2 commits into
ag2ai:mainfrom
zhechen-cs:fix/redis-manual-read-noack
Oct 22, 2026
Merged

IvanKirpichnikov merged 2 commits into
ag2ai:mainfrom
zhechen-cs:fix/redis-manual-read-noack

Conversation

@zhechen-cs

Copy link
Copy Markdown
Contributor

Description

Forward StreamSub.no_ack to XREADGROUP in get_one() and asynchronous iteration. Without it, manual readers configured with no_ack=True still add entries to the consumer group pending list.

Type of change

  • Bug fix

Validation

  • Real Redis regression: 2 failed and 2 passed. Both no_ack=True cases left one pending entry; no_ack=False controls passed.
  • Redis single-node suite: 445 passed, 9 skipped, 30 deselected, 1 xfailed. Final focused run: 5 passed, including the real Redis batch-publish control.
  • Ruff check, Ruff format --check, mypy on both changed files, and git diff --check passed.

Checklist

  • Reviewed the change and its callers
  • Added a regression test
  • Verified the relevant tests within the scope above

The existing documented behavior is preserved; no API changes are introduced.

@github-actions github-actions Bot added the Redis Issues related to `faststream.redis` module and Redis features label Oct 18, 2026
@CLAassistant

CLAassistant commented Oct 20, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

powersemmi
powersemmi previously approved these changes Oct 21, 2026
@pytest.mark.asyncio()
@pytest.mark.parametrize("no_ack", (False, True))
@pytest.mark.parametrize("iterate", (False, True))
async def test_manual_stream_read_respects_no_ack(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s move this test to tests/brokers/redis/test_consume.py and split it into 2 tests: the 1 will be anext(aiter(sub)), the second get_one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @IvanKirpichnikov ! I've moved the tests into TestConsumeStream in test_consume.py and split them into separate get_one() and anext(aiter(sub)) tests, as suggested. Both retain coverage for no_ack=True and no_ack=False.

@IvanKirpichnikov
IvanKirpichnikov added this pull request to the merge queue Oct 22, 2026
Merged via the queue into ag2ai:main with commit 6eab299 Oct 22, 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.

4 participants