add support for default values with a field that is a pydantic model #3499

ppease · 2024-05-14T18:48:32Z

In this PR, I am adding support for having a pydantic model be a default value for a field on an input. Previously this would not generate the proper schema.

Description

My original fix was modifying code in from_input_field inside of the schema converter. This is the point where we are instantiating a graphql input type, and I was checking to see if the default value had a model dump method on it. This seems a little better since it is still within the pydantic specific code. The issue I'm trying to solve is that when we print the schema from within print_input_value we use ast_from_value on the default value. We fall into the is_input_object_type case and then we check if there is an object definition for the value. If is_input_object_type evaluates to True then we call strawberry.asdict. I think we'd want to do something similar with a pydantic instance. Today we fail the check and end up returning None which causes the default value to not be printed in the schema. What I'm doing in this MR is detecting if our default value is a pydantic instance, and if it is then I am storing the serialized value for the pydantic model as the default value.

I'm curious to know if there is a better way to plug into the printing process because that feels more inline with what we are doing with dataclasses today. I could add code that I had previously into either print_input_value or ast_from_value, but that felt a little more hacky since we are putting pydantic specific stuff in higher level code.

Types of Changes

Issues Fixed or Closed by This PR

This fixes #3285

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

strawberry/experimental/pydantic/_compat.py

botberry · 2024-05-14T18:49:44Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Fixes a bug where pydantic models as the default value for an input did not print the proper schema.
See this issue.

Here's the tweet text:

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

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

sourcery-ai

Hey @ppease - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 3 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: 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.}

strawberry/schema/schema_converter.py

sourcery-ai · 2024-05-14T18:50:14Z

strawberry/experimental/pydantic/_compat.py

+if IS_PYDANTIC_V2:
+    import pydantic.v1


question: Clarify the purpose of importing pydantic.v1 under IS_PYDANTIC_V2 condition.

It seems counterintuitive to import a version-specific module conditionally based on the version check. Ensure this is intentional and document the rationale behind this decision.

strawberry/schema/schema_converter.py

codspeed-hq · 2024-05-14T18:56:08Z

CodSpeed Performance Report

Merging #3499 will not alter performance

_{Comparing ppease:feature/default-values (c5f6b60) with main (b7f2881)}

Summary

✅ 12 untouched benchmarks

codecov · 2024-05-14T19:26:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.56%. Comparing base (b7f2881) to head (c5f6b60).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3499   +/-   ##
=======================================
  Coverage   96.56%   96.56%           
=======================================
  Files         523      523           
  Lines       33485    33551   +66     
  Branches     5554     5566   +12     
=======================================
+ Hits        32336    32400   +64     
- Misses        914      916    +2     
  Partials      235      235

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

ppease · 2024-05-14T21:16:00Z

tests/schema/test_input.py

@@ -27,3 +29,72 @@ def filter(self, input: FilterInput) -> str:
    assert not result.errors
    assert result.data
    assert result.data["filter"] == "Hello nope"
+
+
+def test_input_with_nonscalar_field_default():


I originally thought this had issues, but as long as you specify the default factory, this works as intended. I didn't see any tests explicitly testing this so I added one.

patrick91

Sorry for the late reply, I think this looks good!

We still have to figure if we want to remove the default values from the schema or not 🤔 See #3517

But we can merge this either way 😊

strawberry/experimental/pydantic/_compat.py

tests/schema/test_generics_nested.py

patrick91 · 2024-06-10T13:35:22Z

@ppease is there any reason why this is draft? just double checking!

ppease · 2024-06-10T13:46:33Z

@ppease is there any reason why this is draft? just double checking!

@patrick91 This fix resolves my original issue. The only reason I made it a draft is I just wanted to make sure the fix made sense architecturally, and there wasn't a more preferred way to fix the issue.

patrick91 · 2024-06-10T13:53:29Z

@ppease I think the fix is fine! 😊

sourcery-ai

We have skipped reviewing this pull request. It looks like we've already reviewed this pull request.

ppease · 2024-06-10T14:04:16Z

@patrick91 Sounds good! Thanks for reviewing it! I went ahead and marked it as ready.

patrick91 · 2024-06-10T14:53:30Z

@ppease thanks! looks like the tests all passed here, so I merged it

Mypy sometimes does weird things locally with pydantic :D

Patrick Pease and others added 2 commits May 14, 2024 13:45

add support for default values

eae3519

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

e03ae87

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

ppease commented May 14, 2024

View reviewed changes

strawberry/experimental/pydantic/_compat.py Outdated Show resolved Hide resolved

sourcery-ai bot reviewed May 14, 2024

View reviewed changes

ppease commented May 14, 2024

View reviewed changes

strawberry/schema/schema_converter.py Outdated Show resolved Hide resolved

ppease commented May 14, 2024

View reviewed changes

strawberry/schema/schema_converter.py Outdated Show resolved Hide resolved

ppease changed the title ~~add support for default values~~ add support for default values with a field that is a pydantic model May 14, 2024

remove print schema

1a54815

Patrick Pease and others added 2 commits May 14, 2024 16:14

just modify code in pydantic

5bb9a2d

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

bacdbf5

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

ppease commented May 14, 2024

View reviewed changes

ppease marked this pull request as draft May 14, 2024 21:17

Patrick Pease added 6 commits May 14, 2024 16:28

fix url that seems to be updated

f8a678d

remove redundant cast

bf3b721

avoid pydantic imports

12f032f

add release md

055b33d

fix import

be7ea4e

try and remove tag requiring pydantic v2

012f329

patrick91 approved these changes Jun 10, 2024

View reviewed changes

strawberry/experimental/pydantic/_compat.py Outdated Show resolved Hide resolved

tests/schema/test_generics_nested.py Outdated Show resolved Hide resolved

Merge branch 'strawberry-graphql:main' into feature/default-values

fc20cbd

ppease marked this pull request as ready for review June 10, 2024 14:03

sourcery-ai bot reviewed Jun 10, 2024

View reviewed changes

botberry added the bot:has-release-file label Jun 10, 2024

botberry added the bot:release-type-patch label Jun 10, 2024

remove additional import check

c5f6b60

patrick91 merged commit a22a383 into strawberry-graphql:main Jun 10, 2024
64 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for default values with a field that is a pydantic model #3499

add support for default values with a field that is a pydantic model #3499

ppease commented May 14, 2024 •

edited by patrick91

botberry commented May 14, 2024 •

edited

sourcery-ai bot left a comment

sourcery-ai bot May 14, 2024

codspeed-hq bot commented May 14, 2024 •

edited

codecov bot commented May 14, 2024 •

edited

ppease May 14, 2024

patrick91 left a comment

patrick91 commented Jun 10, 2024

ppease commented Jun 10, 2024 •

edited

patrick91 commented Jun 10, 2024

sourcery-ai bot left a comment

ppease commented Jun 10, 2024

patrick91 commented Jun 10, 2024

add support for default values with a field that is a pydantic model #3499

add support for default values with a field that is a pydantic model #3499

Conversation

ppease commented May 14, 2024 • edited by patrick91

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

botberry commented May 14, 2024 • edited

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot May 14, 2024

Choose a reason for hiding this comment

codspeed-hq bot commented May 14, 2024 • edited

CodSpeed Performance Report

Merging #3499 will not alter performance

Summary

codecov bot commented May 14, 2024 • edited

Codecov Report

ppease May 14, 2024

Choose a reason for hiding this comment

patrick91 left a comment

Choose a reason for hiding this comment

patrick91 commented Jun 10, 2024

ppease commented Jun 10, 2024 • edited

patrick91 commented Jun 10, 2024

sourcery-ai bot left a comment

Choose a reason for hiding this comment

ppease commented Jun 10, 2024

patrick91 commented Jun 10, 2024

ppease commented May 14, 2024 •

edited by patrick91

botberry commented May 14, 2024 •

edited

codspeed-hq bot commented May 14, 2024 •

edited

codecov bot commented May 14, 2024 •

edited

ppease commented Jun 10, 2024 •

edited