WIP: Pydantic v2 compat #2888

thejaminator · 2023-06-25T15:08:31Z

This PR provides basic functionality for pydantic v2. I do not recommend people to actually use it, but would like to merge this in first so that others can get work started on it.

This is done by creating a new directory inside strawberry, that is a sister subdirectory to pydantic v1.
This does duplicate a lot of code, but the upside is that

this avoids a lot of if pydantic version == 1 logic
something in pydantic v1 does always have an equivalent in pydantic v2. E.g. the ModelInfo class in pydantic v1 does not have the field name in pydantic v2. In pydantic v2 its a dict instead. So if the code is shared, the v1 code needs to be changed to add this additional information, even though its not needed. so that becomes quite confusing.
pydantic v2 has some different resolution logic for types than v1
if pydantic version == 1 at run time messes up your IDE and type checkers. (or at least it did for me)

FastAPI did take the other approach of having a compat module, and a bunch of if pydantic version == 1 logic scattered around tests and files. I'm not sure which of these ways are better. Having a shared module like them could have been better if we really are commiting to continue to support pydantic v1 when we introduce new fixes. But I do think that it would slow us down in the sense of developing something new on pydantic v2 (because we are scared to break pydantic v1 stuff)

For tests, i've only added some of them so far. I'll add the rest of them in from pydantic v1 over the next few MRs. Not all the tests have the same behavior between pydantic v1 and v2.

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

for more information, see https://pre-commit.ci

codecov · 2023-06-25T15:09:38Z

Codecov Report

Merging #2888 (227736d) into main (a1dae33) will decrease coverage by 1.48%.
The diff coverage is 55.27%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2888      +/-   ##
==========================================
- Coverage   96.11%   94.64%   -1.48%     
==========================================
  Files         211      219       +8     
  Lines        9232     9591     +359     
  Branches     1489     1562      +73     
==========================================
+ Hits         8873     9077     +204     
- Misses        229      367     +138     
- Partials      130      147      +17

thejaminator · 2023-06-25T15:10:21Z

strawberry/experimental/pydantic2/object_type.py

+    use_pydantic_alias: bool = True,
+) -> Callable[..., Type[StrawberryTypeFromPydantic[PydanticModel]]]:
+    def wrap(cls: Any) -> Type[StrawberryTypeFromPydantic[PydanticModel]]:
+        model_fields: Dict[str, FieldInfo] = model.model_fields


in pydantic v1 this was ModelFields, but its now FieldInfo in pydantic v2. The FieldInfo also does not contain the name of the field itself, so now its a dit.

botberry · 2023-06-25T15:10:46Z

Hi, thanks for contributing to Strawberry 🍓!

We noticed that this PR is missing a RELEASE.md file. We use that to automatically do releases here on GitHub and, most importantly, to PyPI!

So as soon as this PR is merged, a release will be made 🚀.

Here's an example of RELEASE.md:

Release type: patch

Description of the changes, ideally with some examples, if adding a new feature.

Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :)

Here's the tweet text:

🆕 Release (next) is out! Thanks to James Chua for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

thejaminator · 2023-06-25T15:15:59Z

tests/experimental/pydantic2/schema/test_basic.py

+        age: int
+        password: Optional[str]
+
+    @strawberry.experimental.pydantic2.type(UserModel, all_fields=True)


for now its just pydantic2, for the lack of a better name

for more information, see https://pre-commit.ci

thejaminator · 2023-07-07T09:33:19Z

strawberry/experimental/pydantic2/utils.py

+
+    # Note that unlike pydantic v1, pydantic v2 does not add a default of None when
+    # the field is Optional[something]
+    # so there is no need to handle that case here


in v1 there was some logic of adding a default of None when its Optional. pydantic v2 doesn't do that anymore

for more information, see https://pre-commit.ci

codspeed-hq · 2023-07-12T19:11:00Z

CodSpeed Performance Report

Merging #2888 will not alter performance

_{⚠️ No base runs were found}

_{Falling back to comparing thejaminator:pydantic-v2-compat (227736d) with main (efcd602)}

Summary

✅ 1 untouched benchmarks

for more information, see https://pre-commit.ci

thejaminator · 2023-07-13T01:32:57Z

noxfile.py

        "--ignore=tests/mypy",
        "--ignore=tests/pyright",
+        "--ignore=tests/experimental/pydantic",
+        "--ignore=tests/experimental/pydantic2",
    )


otherwise pytest on the pydantic2 folder would go boom. since it tries to import stuff directly from pydantic v2.

thejaminator · 2023-07-13T01:33:47Z

strawberry/experimental/__init__.py

+    # We don't recommend using it yet
+    __all__ = ["pydantic2"]
+except ImportError as e:
+    pass


explicitly a separate module

for more information, see https://pre-commit.ci

ppease · 2023-07-20T19:11:13Z

@thejaminator I was just curious if we had any idea on timeline for when we would feel comfortable with someone using pydantic v2 in a production type environment? I know your comment above mentioned that this MR is just adding initial support, and we wouldn't recommend people using it.

Not sure this is the right place to ask, but I was also wondering if there has been any talk of removing the experimental tag from pydantic in general? Our team has found a ton of value out of using the pydantic integration with strawberry. For better or worse we've been using the existing experimental pydantic support in our production environments for over a year without any issues.

patrick91 · 2023-07-21T06:52:42Z

@thejaminator I was just curious if we had any idea on timeline for when we would feel comfortable with someone using pydantic v2 in a production type environment? I know your comment above mentioned that this MR is just adding initial support, and we wouldn't recommend people using it.

I’m going to chat with James next week about this :)

Not sure this is the right place to ask, but I was also wondering if there has been any talk of removing the experimental tag from pydantic in general? Our team has found a ton of value out of using the pydantic integration with strawberry. For better or worse we've been using the existing experimental pydantic support in our production environments for over a year without any issues.

That’s good to know, I think we could do that, but we’d need to make sure we have enough bandwidth to fix issues once this is marked as non experimental

there’s also an idea of doing first class support for pydantic in future, so maybe this integration could not be that useful anymore?

Also @ppease do you think your company could be interested in sponsoring us to work a bit on this? that’d be very helpful ☺️

ppease · 2023-07-21T14:20:13Z

Thanks for the quick response. Is this the right issue to follow to get updates on work being done with pydantic v2? Also I'd be willing to try an early version of stuff and report any bugs if that would help you all out.

there’s also an idea of doing first class support for pydantic in future, so maybe this integration could not be that useful anymore?

I'm assuming this issue is what you mean by first class support? If so I do think first class support would help us because that is one of the pain points right now with using the experimental support is having to create duplicate types. There will probably still be cases where we end up creating duplicate classes because we want to expose different items at the graphql layer vs. our data model layer so maybe the experimental support would still be useful for those use cases? I could also see different ways of doing that with a more first class built in way as well.

Also @ppease do you think your company could be interested in sponsoring us to work a bit on this? that’d be very helpful

We are not too big of a company so I can't make any promises, but I'll bring it up. At a minimum we might be able to commit some time from myself or another dev to try and help out as well.

thejaminator · 2023-07-22T22:57:45Z

Making the MR here isntead (did a compat file instead of a different folder)
#2972

patrick91 · 2023-07-24T18:09:05Z

@ppease can you try this release? strawberry-graphql==0.196.0.dev.1690222024

ppease · 2023-07-27T18:51:34Z

@patrick91 I just saw this. I probably won't have time this week, and I'm OOO next week. However, when I get back, I should be able to prioritize trying this out.

cvarner-shippo · 2023-08-07T17:56:52Z

Hi there, I'm currently running strawberry-graphql==0.196.0.dev.1690222024. Is the following expected behavior?

@strawberry.experimental.pydantic.type(model=PydanticOne, all_fields=True)
class One:
    @strawberry.field
    def two(self: 'One') -> 'Two':
        return Two()

@strawberry.experimental.pydantic.type(model=PydanticTwo, all_fields=True)
class Two:
    @strawberry.field
    def one(self: 'Two') -> 'One':
        return One()

Type hints work as expected, but at runtime whichever type is defined second fails to resolve the forward ref on the strawberry.field.

strawberry.exceptions.unresolved_field_type.UnresolvedFieldTypeError: Could not resolve the type of 'one'. Check that the class is accessible from the global module scope.

Side note, this error message was difficult to debug as nowhere in the stack trace does it specify the filename or class the unresolved field belongs to.

I can use LazyType to make this work but it mucks up type hints and feels unnecessary (considering the type hints were working before).

@strawberry.experimental.pydantic.type(model=PydanticOne, all_fields=True)
class Two:
    @strawberry.field
    def one(self: 'Two') -> LazyType['One', __name__]:
        return One()

ppease · 2023-09-17T16:01:30Z

@patrick91 Sorry for taking a while to respond to this. I tried the latest version of strawberry, 0.208.1, with my graphql project, and I was able to generate a schema that matched my existing schema aside from some stuff moving around which was related to alphabetical order. I did have to comment out two fields. One that was using conint, and another one that was using EmailStr. It looks like inside of strawberry/experimental/pydantic/fields.py we already have a FIELDS_MAP to map from pydantic specific types to the base type. If both of those fields are fully supported in pydantic v2, could I just add those to the pydantic v2 field map?

Another data point is I ran all of our unit tests for our graphql code and those passed as well so this seems like a really promising start! I plan on doing more testing in our dev environment this week, and I'll report back any issues I find.

patrick91 · 2023-09-17T17:35:42Z

@ppease feel free to open issues or send PRs 😊

thejaminator and others added 9 commits May 20, 2023 17:46

work so far

ee45b97

separate

4c13e84

remove field map

95f979a

yay test pass

ddf7ade

fix fields

9734494

fix passing fields

2b2c1a8

it worksgit stage .

df87fb7

revert

017e168

[pre-commit.ci] auto fixes from pre-commit.com hooks

2103eab

for more information, see https://pre-commit.ci

thejaminator commented Jun 25, 2023

View reviewed changes

remove compat file

064db83

thejaminator commented Jun 25, 2023

View reviewed changes

thejaminator added 3 commits July 7, 2023 17:10

fix pydantic v2 update issues

274f2b6

Merge remote-tracking branch 'origin/main' into pydantic-v2-compat

ae295f5

add test for strawberry-graphql#2782

f582d41

thejaminator force-pushed the pydantic-v2-compat branch from fee8e8d to f582d41 Compare July 7, 2023 09:29

[pre-commit.ci] auto fixes from pre-commit.com hooks

045856a

for more information, see https://pre-commit.ci

thejaminator commented Jul 7, 2023

View reviewed changes

Merge branch 'main' into pydantic-v2-compat

590166d

patrick91 mentioned this pull request Jul 8, 2023

Add pydantic tests via nox #2935

Merged

thejaminator and others added 5 commits July 11, 2023 10:04

Merge remote-tracking branch 'origin/main' into pydantic-v2-compat

176dcbf

mark pydantic v2 explicitly

5139f7a

[pre-commit.ci] auto fixes from pre-commit.com hooks

0e0d1a5

for more information, see https://pre-commit.ci

add pytest markers for pydantic_v2

8fa12dc

try again

c7a9b8c

thejaminator force-pushed the pydantic-v2-compat branch from eb59415 to c7a9b8c Compare July 11, 2023 18:59

thejaminator added 2 commits July 12, 2023 11:57

add ignore pydantic2

6be92be

add explicit dir

fecd02d

[pre-commit.ci] auto fixes from pre-commit.com hooks

883b8a4

for more information, see https://pre-commit.ci

thejaminator and others added 4 commits July 12, 2023 12:29

remove markers

e6fd48b

[pre-commit.ci] auto fixes from pre-commit.com hooks

858998c

for more information, see https://pre-commit.ci

add ignore for test

ae9eff1

add hints

42203c6

thejaminator commented Jul 13, 2023

View reviewed changes

thejaminator and others added 2 commits July 12, 2023 20:06

fix weird cli tests changes

3d97786

[pre-commit.ci] auto fixes from pre-commit.com hooks

227736d

for more information, see https://pre-commit.ci

g-as mentioned this pull request Jul 13, 2023

Pedantic v2.0.2 is not supported #2946

Closed

thejaminator closed this Jul 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Pydantic v2 compat #2888

WIP: Pydantic v2 compat #2888

thejaminator commented Jun 25, 2023 •

edited

Loading

codecov bot commented Jun 25, 2023 •

edited

Loading

thejaminator Jun 25, 2023

botberry commented Jun 25, 2023 •

edited

Loading

thejaminator Jun 25, 2023

thejaminator Jul 7, 2023 •

edited

Loading

codspeed-hq bot commented Jul 12, 2023 •

edited

Loading

thejaminator Jul 13, 2023

thejaminator Jul 13, 2023

ppease commented Jul 20, 2023

patrick91 commented Jul 21, 2023

ppease commented Jul 21, 2023

thejaminator commented Jul 22, 2023

patrick91 commented Jul 24, 2023

ppease commented Jul 27, 2023

cvarner-shippo commented Aug 7, 2023

ppease commented Sep 17, 2023

patrick91 commented Sep 17, 2023

WIP: Pydantic v2 compat #2888

WIP: Pydantic v2 compat #2888

Conversation

thejaminator commented Jun 25, 2023 • edited Loading

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

codecov bot commented Jun 25, 2023 • edited Loading

Codecov Report

thejaminator Jun 25, 2023

Choose a reason for hiding this comment

botberry commented Jun 25, 2023 • edited Loading

thejaminator Jun 25, 2023

Choose a reason for hiding this comment

thejaminator Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

codspeed-hq bot commented Jul 12, 2023 • edited Loading

CodSpeed Performance Report

Merging #2888 will not alter performance

Summary

thejaminator Jul 13, 2023

Choose a reason for hiding this comment

thejaminator Jul 13, 2023

Choose a reason for hiding this comment

ppease commented Jul 20, 2023

patrick91 commented Jul 21, 2023

ppease commented Jul 21, 2023

thejaminator commented Jul 22, 2023

patrick91 commented Jul 24, 2023

ppease commented Jul 27, 2023

cvarner-shippo commented Aug 7, 2023

ppease commented Sep 17, 2023

patrick91 commented Sep 17, 2023

thejaminator commented Jun 25, 2023 •

edited

Loading

codecov bot commented Jun 25, 2023 •

edited

Loading

botberry commented Jun 25, 2023 •

edited

Loading

thejaminator Jul 7, 2023 •

edited

Loading

codspeed-hq bot commented Jul 12, 2023 •

edited

Loading