Skip to content

fix(cache): redis backend uses async clients + real-redis CI#725

Merged
wbarnha merged 1 commit into
claude/fix-715-cibuildwheel-314from
claude/redis-async-client-and-ci
Jul 21, 2026
Merged

fix(cache): redis backend uses async clients + real-redis CI#725
wbarnha merged 1 commit into
claude/fix-715-cibuildwheel-314from
claude/redis-async-client-and-ci

Conversation

@wbarnha

@wbarnha wbarnha commented Jul 20, 2026

Copy link
Copy Markdown
Member

Description

Answers "can we test redis in CI?" — yes, and doing so surfaced that the redis cache backend was broken against a real server. This fixes it and adds the real-redis CI leg. Independent, off master. Supersedes #724 (its guard fix + test rewrite are folded in here; #724 is closed).

The bug (proven against a live redis)

The backend maps schemes to the synchronous redis.StrictRedis / redis.RedisCluster, but _get/_set/_delete/connect await the client:

>>> async with app.cache:
...     await app.cache.set("k", b"v", timeout=60)
TypeError: object bool can't be used in 'await' expression

It only ever looked fine because every unit test mocks the client with AsyncMock.

Fix — faust/web/cache/backends/redis.py

  • Use the asyncio clients (redis.asyncio.StrictRedis / redis.asyncio.RedisCluster) so the awaited calls are real coroutines. Verified end-to-end against a live redis (set → get → overwrite → delete round-trip).
  • Close the client on stop (on_stopawait client.aclose()) — the async client holds real connections.
  • Bind redis = None (not just aredis) on ImportError, so the "not installed" guard raises ImproperlyConfigured instead of NameError.

Tests

  • mocked_redis fixture now patches redis.asyncio.StrictRedis (the name the backend uses) and provides aclose.
  • Test_RedisScheme + the not-installed test re-enabled, asserting the async client wiring.
  • New real-redis integration tests (tests/integration/cache/), env-gated on FAUST_TEST_REDIS (skip when no server is reachable, like the broker suite), exercising set/get/overwrite/delete/missing against a live server. A conftest fixture reaps the loop's default-executor thread so the strict hygiene guards stay green.

CI

New test-redis-integration job with a redis:7 service container runs the integration tests against a real server. Advisory (not in the required check gate), matching the Kafka integration job — easy to promote to required later since, unlike the broker, redis is deterministic.

Verification

Unit test_cache.py (32) + integration tests/integration/cache (2) → 34 passed; the integration tests skip cleanly with no reachable redis; flake8/black/isort clean. redis is already in requirements/test.txt (via extras/redis.txt), so the CI job installs it.

🤖 Generated with Claude Code


Generated by Claude Code

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.84%. Comparing base (aa75db0) to head (7015cb1).

Files with missing lines Patch % Lines
faust/web/cache/backends/redis.py 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                         Coverage Diff                         @@
##           claude/fix-715-cibuildwheel-314     #725      +/-   ##
===================================================================
+ Coverage                            94.81%   94.84%   +0.02%     
===================================================================
  Files                                  104      104              
  Lines                                11154    11158       +4     
  Branches                              1202     1203       +1     
===================================================================
+ Hits                                 10576    10583       +7     
+ Misses                                 482      479       -3     
  Partials                                96       96              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The redis cache backend was broken against a real server: it maps schemes to
the *synchronous* redis.StrictRedis / redis.RedisCluster clients but `_get`/
`_set`/`_delete`/`connect` await them, so `await self.client.get(...)` raised
"object ... can't be used in 'await' expression".  It only appeared to work
because every test mocks the client.

* Use the asyncio clients (redis.asyncio.StrictRedis / .RedisCluster), so the
  awaited calls are real coroutines.  Verified end-to-end against a live redis
  (set/get/delete round-trip).
* Close the client on stop (`on_stop` -> `await client.aclose()`); the async
  client holds real connections.
* Bind `redis = None` (not just `aredis`) on ImportError so the "not installed"
  guard raises ImproperlyConfigured instead of NameError.
* mocked_redis fixture: patch redis.asyncio.StrictRedis (the name the backend
  now uses) and give the mock `aclose`.
* Re-enable Test_RedisScheme / the not-installed test, asserting the async
  client wiring.

Add a real-redis CI leg: a `test-redis-integration` job with a redis service
container runs new env-gated integration tests (tests/integration/cache) that
exercise the backend against a live server -- the coverage the mocks can't
give.  Advisory (not in the required `check` gate), matching the Kafka job.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
@wbarnha
wbarnha force-pushed the claude/redis-async-client-and-ci branch from 21a4c69 to 7015cb1 Compare July 21, 2026 00:02
@wbarnha
wbarnha changed the base branch from master to claude/fix-715-cibuildwheel-314 July 21, 2026 00:02
@wbarnha
wbarnha force-pushed the claude/redis-async-client-and-ci branch from cf8c97f to 7015cb1 Compare July 21, 2026 00:16
@wbarnha
wbarnha merged commit e0bdeb3 into claude/fix-715-cibuildwheel-314 Jul 21, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant