Skip to content

Conversation

@danfimov
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings November 28, 2025 15:55
@danfimov danfimov force-pushed the fix-argument-parsing-in-admin-middleware branch from 0cf4e30 to f3f57d7 Compare November 28, 2025 15:56
Copilot finished reviewing on behalf of danfimov November 28, 2025 15:58
@danfimov danfimov force-pushed the fix-argument-parsing-in-admin-middleware branch from f3f57d7 to 8a21f11 Compare November 28, 2025 15:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where TaskiqAdminMiddleware was failing when task arguments contained dataclasses or other non-pydantic types. The fix converts TaskiqMessage and TaskiqResult objects to dictionaries using model_dump() before extracting fields for JSON serialization, ensuring proper serialization of nested objects.

Key changes:

  • Modified TaskiqAdminMiddleware to use model_dump() on message and result objects before accessing their fields
  • Updated taskiq/compat.py to modernize type hints and fix a condition ordering issue
  • Replaced the old admin middleware test file with a new comprehensive test suite that verifies serialization of dataclasses, TypedDicts, and Pydantic models

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
taskiq/middlewares/taskiq_admin_middleware.py Fixed serialization by converting messages/results to dicts before field access
taskiq/compat.py Modernized type hints and fixed version comparison condition
tests/middlewares/test_taskiq_admin_middleware.py Removed old test file
tests/middlewares/admin_middleware/test_arguments_formatting.py Added new parameterized tests for different DTO types
tests/middlewares/admin_middleware/dto.py Added test DTOs (dataclass, TypedDict, Pydantic)
tests/middlewares/admin_middleware/conftest.py Added test fixtures for mock admin server and broker setup
pyproject.toml Added polyfactory dependency for test data generation
uv.lock Updated lock file with new dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@danfimov danfimov force-pushed the fix-argument-parsing-in-admin-middleware branch from 8a21f11 to 253624d Compare November 28, 2025 16:02
@danfimov danfimov changed the title fix: Taskiqmiddleware not working with dataclasses fix: TaskiqAdminMiddleware doesn't work with dataclasses Nov 28, 2025
@danfimov danfimov marked this pull request as draft November 28, 2025 16:10
@danfimov danfimov force-pushed the fix-argument-parsing-in-admin-middleware branch from ae3180a to 9e0c8e8 Compare November 28, 2025 16:23
@danfimov danfimov force-pushed the fix-argument-parsing-in-admin-middleware branch from 9e0c8e8 to 8c053fe Compare November 28, 2025 16:26
@danfimov danfimov marked this pull request as ready for review November 28, 2025 16:33
@danfimov danfimov requested a review from s3rius November 29, 2025 15:30
@danfimov danfimov self-assigned this Nov 29, 2025
@s3rius s3rius merged commit 87d5394 into master Nov 30, 2025
54 checks passed
@danfimov danfimov deleted the fix-argument-parsing-in-admin-middleware branch December 1, 2025 09:20
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.

3 participants