-
-
Notifications
You must be signed in to change notification settings - Fork 544
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
feature/pydantic 1 and 2 #3426
feature/pydantic 1 and 2 #3426
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3426 +/- ##
=======================================
Coverage 96.43% 96.43%
=======================================
Files 507 508 +1
Lines 32178 32272 +94
Branches 5325 5344 +19
=======================================
+ Hits 31030 31122 +92
- Misses 937 939 +2
Partials 211 211 |
CodSpeed Performance ReportMerging #3426 will not alter performanceComparing Summary
|
/pre-release |
Pre-release👋 Releasing commit [fd7e83a] to PyPi as pre-release! 📦 |
Pre-release👋 Pre-release 0.224.0.dev.1711748192 [fd7e83a] has been released on PyPi! 🚀 poetry add strawberry-graphql==0.224.0.dev.1711748192 |
Thanks for adding the Here's a preview of the changelog: This release adds support for using both Pydantic v1 and v2, when importing from This is automatically detected and the correct version is used. Here's the tweet text:
|
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.
Hey @patrick91 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
_missing_type: Any | ||
is_v1: bool |
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.
suggestion (code_clarification): Consider documenting the purpose of _missing_type
and is_v1
attributes.
@@ -0,0 +1,83 @@ | |||
import textwrap |
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.
suggestion (testing): Consider adding negative test cases.
It's great to see positive test scenarios covered for both Pydantic v1 and v2 compatibility. However, adding negative test cases where the schema should fail (e.g., invalid types or missing required fields) would further ensure robustness.
@@ -846,6 +840,7 @@ | |||
assert user.passwords == ["hunter2"] | |||
|
|||
|
|||
@pytest.mark.xfail |
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.
suggestion (testing): Clarify reason for expected failure.
Using @pytest.mark.xfail
indicates that this test is expected to fail, but it's not immediately clear why. Please add a reason to the decorator to clarify the expected failure condition for future reference.
Closes #3425