Skip to content
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

Merged
merged 12 commits into from
Mar 30, 2024
Merged

feature/pydantic 1 and 2 #3426

merged 12 commits into from
Mar 30, 2024

Conversation

patrick91
Copy link
Member

@patrick91 patrick91 commented Mar 29, 2024

Closes #3425

Copy link

codecov bot commented Mar 29, 2024

Codecov Report

Merging #3426 (4a1a498) into main (905b02e) will increase coverage by 0.00%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

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           

Copy link

codspeed-hq bot commented Mar 29, 2024

CodSpeed Performance Report

Merging #3426 will not alter performance

Comparing feature/pydantic-1-and-2 (4a1a498) with main (905b02e)

Summary

✅ 13 untouched benchmarks

@patrick91
Copy link
Member Author

/pre-release

@botberry
Copy link
Member

Pre-release

👋

Releasing commit [fd7e83a] to PyPi as pre-release! 📦

@botberry
Copy link
Member

Pre-release

👋

Pre-release 0.224.0.dev.1711748192 [fd7e83a] has been released on PyPi! 🚀
You can try it by doing:

poetry add strawberry-graphql==0.224.0.dev.1711748192

@patrick91 patrick91 marked this pull request as ready for review March 30, 2024 12:43
@botberry
Copy link
Member

botberry commented Mar 30, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds support for using both Pydantic v1 and v2, when importing from
pydantic.v1.

This is automatically detected and the correct version is used.

Here's the tweet text:

🆕 Release (next) is out! Thanks to @patrick91 for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a 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

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Comment on lines +33 to +34
_missing_type: Any
is_v1: bool
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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.

strawberry/experimental/pydantic/_compat.py Show resolved Hide resolved
strawberry/experimental/pydantic/_compat.py Show resolved Hide resolved
strawberry/experimental/pydantic/_compat.py Show resolved Hide resolved
strawberry/experimental/pydantic/_compat.py Show resolved Hide resolved
strawberry/experimental/pydantic/_compat.py Show resolved Hide resolved
strawberry/experimental/pydantic/utils.py Show resolved Hide resolved
@patrick91 patrick91 merged commit e7d7b5a into main Mar 30, 2024
62 of 63 checks passed
@patrick91 patrick91 deleted the feature/pydantic-1-and-2 branch March 30, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support both Pydantic 1 and 2
2 participants