Conversation
Instead of proactively creating the Redis stream and consumer group on every `async with Docket(...)`, we now create them lazily when actually needed. This avoids the noisy BUSYGROUP errors that were showing up in production logs. The worker now catches NOGROUP errors from xautoclaim/xreadgroup and creates the group on demand. Similarly, snapshot() handles NOGROUP (with a workaround for a redis-py 7.0 bug where pipeline errors lose the original message). For the memory:// backend (fakeredis), we proactively create the group in snapshot() since fakeredis has a bug where it raises TypeError instead of NOGROUP. Closes #206 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @chrisguidry's task —— View job Code Review Complete ✅I've completed my review of PR #215 implementing lazy stream/consumer group bootstrap. This is a well-architected solution that addresses production issues while maintaining backward compatibility. Review Checklist:
SummaryOverall Assessment: ✅ Approved with minor suggestions This PR successfully implements lazy initialization of Redis streams and consumer groups, moving from proactive creation to on-demand creation. This eliminates noisy BUSYGROUP errors in production logs while maintaining robust error handling. Strengths1. Clean Architecture 🏗️
2. Comprehensive Error Handling 🛡️
3. Excellent Test Coverage ✅
Potential Improvements1. Minor: Consider Retry Logic EnhancementIn async def get_redeliveries(redis: Redis, retry_count: int = 0) -> RedisReadGroupResponse:
if retry_count > 3:
raise RuntimeError("Failed to create consumer group after multiple attempts")
# ... existing code ...2. Documentation: Add Migration NoteConsider adding a comment in 3. Chaos Driver EnhancementGood addition of NOGROUP handling in chaos/driver.py:250-255. This ensures the chaos testing continues to work with the new lazy initialization. Performance Considerations ⚡✅ Positive Impact:
Security Assessment 🔒✅ No security concerns identified:
Code Quality 📊✅ High quality implementation:
Minor Observations
ConclusionThis is a solid implementation that solves a real production issue (#206) elegantly. The lazy initialization pattern is the right approach here, and the implementation is robust with proper error handling and test coverage. The code is production-ready. Recommendation: Merge ✅ Great work on this improvement! The lazy initialization will make Docket more efficient in production environments. |
|
📚 Documentation has been built for this PR! You can download the documentation directly here: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #215 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 48 48
Lines 7600 7740 +140
Branches 397 402 +5
==========================================
+ Hits 7600 7740 +140
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Replace asyncio.TaskGroup with asyncio.gather for Python 3.10 compat - Handle NOGROUP errors in chaos driver when workers haven't started yet 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
Instead of proactively creating the Redis stream and consumer group on every
async with Docket(...), we now create them lazily when actually needed. Thisavoids the noisy BUSYGROUP errors that were showing up in production logs.
The worker now catches NOGROUP errors from xautoclaim/xreadgroup and creates
the group on demand. Similarly, snapshot() handles NOGROUP (with a workaround
for a redis-py 7.0 bug where pipeline errors lose the original message).
For the memory:// backend (fakeredis), we proactively create the group in
snapshot() since fakeredis has a bug where it raises TypeError instead of
NOGROUP.
Closes #206
🤖 Generated with Claude Code