-
Notifications
You must be signed in to change notification settings - Fork 5
[Refactor] Fix Saga and StramHandler typing #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Docstrings generation was requested by @vadikko2. * #52 (comment) The following files were modified: * `benchmarks/dataclasses/test_benchmark_cor_request_handler.py` * `benchmarks/dataclasses/test_benchmark_event_handling.py` * `benchmarks/dataclasses/test_benchmark_request_handling.py` * `benchmarks/dataclasses/test_benchmark_saga_fallback.py` * `benchmarks/dataclasses/test_benchmark_saga_memory.py` * `benchmarks/dataclasses/test_benchmark_saga_sqlalchemy.py` * `benchmarks/dataclasses/test_benchmark_stream_request_handler.py` * `benchmarks/pydantic/test_benchmark_cor_request_handler.py` * `benchmarks/pydantic/test_benchmark_event_handling.py` * `benchmarks/pydantic/test_benchmark_request_handling.py` * `benchmarks/pydantic/test_benchmark_saga_fallback.py` * `benchmarks/pydantic/test_benchmark_saga_memory.py` * `benchmarks/pydantic/test_benchmark_saga_sqlalchemy.py` * `benchmarks/pydantic/test_benchmark_serialization.py` * `benchmarks/pydantic/test_benchmark_stream_request_handler.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
# Conflicts: # tests/benchmarks/dataclasses/test_benchmark_stream_request_handler.py # tests/benchmarks/pydantic/test_benchmark_stream_request_handler.py
📝 WalkthroughWalkthroughThe changes implement an architectural refactor converting async streaming methods to synchronous functions returning AsyncIterator objects across request handlers, dispatchers, and mediators. Type-ignore directives are removed as the new pattern eliminates type checking conflicts. Examples and tests are updated accordingly, and the version is bumped to 4.7.3. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #55 +/- ##
==========================================
+ Coverage 83.93% 83.99% +0.05%
==========================================
Files 70 70
Lines 2428 2436 +8
==========================================
+ Hits 2038 2046 +8
Misses 390 390 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@examples/streaming_handler_parallel_events.py`:
- Around line 444-452: The summary assertions and logs for results,
EMAIL_SENT_LOG, AUDIT_LOG, and ANALYTICS_STORAGE are running before the
fire-and-forget event handler tasks finish; move the await asyncio.sleep(0.3) so
it executes before the assertions/summary block that checks len(results),
len(EMAIL_SENT_LOG), len(AUDIT_LOG) and ANALYTICS_STORAGE["total_orders"]
against order_ids; this ensures the create_task-created handlers (EventProcessor
when concurrent_event_handle_enable=True) have time to complete before
verification.
In `@src/cqrs/saga/bootstrap.py`:
- Around line 188-190: The example iterates mediator.stream(order_context) and
incorrectly accesses result.step_type.__name__; update it to read the nested
property on SagaDispatchResult by using result.step_result.step_type.__name__
instead (SagaDispatchResult.step_result -> SagaStepResult.step_type) so the code
references the correct attribute path when printing step names.
🧹 Nitpick comments (3)
examples/fastapi_sse_streaming.py (3)
233-233: Consider using a single f-string.The implicit concatenation of two f-strings works but is unnecessarily verbose. A single f-string would be cleaner.
✨ Suggested simplification
logger.info( - f"Starting to process {len(request.file_ids)} files " f"with operation: {request.operation}", + f"Starting to process {len(request.file_ids)} files with operation: {request.operation}", )
310-310: Same pattern - consider a single f-string.✨ Suggested simplification
logger.info( - f"📄 File {event.file_id} processed: " f"{event.operation} ({event.file_size_mb} MB)", + f"📄 File {event.file_id} processed: {event.operation} ({event.file_size_mb} MB)", )
321-321: Same pattern - consider a single f-string.✨ Suggested simplification
logger.info( - f"📊 Analytics updated for file {event.file_id}: " f"{event.processing_time_ms}ms processing time", + f"📊 Analytics updated for file {event.file_id}: {event.processing_time_ms}ms processing time", )
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Created PR with unit tests: #56 |
Summary by CodeRabbit
Release v4.7.3
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.