-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: upgrade to pydantic v2 #160
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #160 +/- ##
==========================================
+ Coverage 61.94% 68.82% +6.88%
==========================================
Files 37 45 +8
Lines 938 1360 +422
==========================================
+ Hits 581 936 +355
- Misses 357 424 +67
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@thoas, hi and thanks for this pull request. Tests pass fine, but this PR contains some minor styling issues. You can automatically fix them using command I really appreciate your contribution, but I have a question. Can we support both pydanticV1 and pydanticV2? Because not all users will migrate to newer version. |
Thank you for the review, I will update the styling and you are right we should keep the support for pydantic v1, I will work on that. |
@s3rius I have added a compatibility layer for both pydantic v1 and v2. Let me know if you need anything else. PS: I think you should authorize by default github actions |
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.
I really like the way you did it. But I have some suggestions to make it even 🌠 better 🌠 .
Hope you're going to like it. I really appreciate your contributions. Thanks.
@thoas, about gh actions. You'll be able to run it automatically after first merged PR. Hope it will happen soon, btw. ❤️ |
cd86248
to
ea90e72
Compare
I made the changes, thank you again for your review and don't hesitate. |
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.
Looks really good. One small issue with pydantic version. With current constraint it won't allow you to install pydantic with version bigger than 2.0
.
fixed and I also fixed the typing compatibility for Python 3.9 |
... and here we go again for Python 3.8 ❤️ sorry for the noise (I only work with Python 3.11) |
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.
Congratulation with your first merged request in taskiq. Thank you for your contribution!
I will release it after we merge some other requests. |
Thank you @s3rius for your time and review, I can now run gh actions by myself! |
Tests passed locally but there are still two warnings:
I can't get
pydantic.validate_call
to work properly, it fails with this traceback:and it annoys me 😒