-
Notifications
You must be signed in to change notification settings - Fork 44
Pydantic V2 Support #94
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
# Conflicts: # graphene_pydantic/converters.py # poetry.lock
deprecated: pydantic v1
This version marks support of Pydantic V2 and python 3.12, and drops support for Pydantic V1
Thank you @mak626 for the PR! Obviously this changes the valid dependencies for
@dantheman39, do you agree? |
@necaris Maybe a typo on your side, but just to clarify support for Python support for 3.7 and below is only dropped, as it's EOL. We can maintain support for Python >=3.8 |
Thank you for your work! Just commenting here to express my interest in seeing this get merged / released properly. |
I think I just found a bug in this @mak626 @abhinand-c:
|
@ported-pw , pydantic v2 has changed the way we use fields The is the current way to use the fields to achieve optional ![]() Reference: https://docs.pydantic.dev/2.0/migration/ |
Sorry @mak626 , maybe my examples were not clear enough, I'll extend them to proper fields:
None of these would be marked optional in the GraphQL schema right now, while they are not required by Pydantic during model validation, leading to this error: There does not seem to be a test case for this combination of explicit union + optional/union with |
Thanks, we missed this case. Have fixed this now. |
Awesome, seems to work well now :) |
@necaris @mak626 would a major version bump be more appropriate? Does this include breaking changes or am I mistaken? But I agree with creating a maintenance branch, and also agree with submitting a follow-up cleanup PR. I have a bit of time coming up and would be happy to do that. Are you thinking we merge this as-is, then? |
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.
Thanks so much for this contribution. I'm currently traveling but will try to find time soon to give a proper review.
Question: should any updates be made to the README?
I ran the test suite and it looks like CI is not passing. If you're able to take a look at why that will be great.
Thanks again!
Yes, this would include a breaking change dropping support for Pydantic 1.x (since the changes are incompatible); I'm not against making a major version bump, although IMHO calling any part of this library 1.x (implying readiness for production) is premature... 😉 |
Yes it includes breaking changes as pydantic v2 usage is a lot different from v1. Major version bump would be necessary. Also this library would work for python versions >=3.8 not just >=3.10 |
Yes I have fixed the tests now for other python versions. Was due to an import error for UnionType of python 10. Readme can be updated to emphasise that there are breaking changes when switching from v1 to v2 along with this link https://docs.pydantic.dev/2.0/migration/ Other than that I think the way we use this library remains the same. |
260097d
to
ed997c9
Compare
ed997c9
to
58135e7
Compare
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.
Thank you!
Fixes #87
Changelog
pydantic v1
andpython < 3.8
.pydantic v2
ModelField
->FieldInfo
pydantic.parse_obj_as(FooBar, data["listFooBars"])
was changed to new syntaxTypeAdapter(FooBar).validate_python(data["createFooBar"])
ObjectId
->graphene.ID
dict
->JSONString
is_type_of
resolvers. This simplifies usingUnionType
asis_type_of
need not be defined individually in the base types ofUnion
Pydantic V2 also changed the way we define fields. Please refer their doc Migration Guide. Some of the breaking changes include
Optional[type]
no longer has a default valueNone
unless given explicitly.