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

[strawberry.ext.mypy_plugin] Fix import error for users who don't use pydantic #3018

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

davidnemec
Copy link
Contributor

This should fix import error in strawberry.ext.mypy_plugin for users who don't use pydantic introduced in latest version. Hopefully the new location of the import is fine.

Error importing plugin "strawberry.ext.mypy_plugin": No module named 'pydantic'

@botberry
Copy link
Member

botberry commented Aug 8, 2023

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Fix import error in strawberry.ext.mypy_plugin for users who don't use pydantic.


Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to David Němec for the PR 👏

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

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #3018 (ad2dbb8) into main (b456a70) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3018      +/-   ##
==========================================
- Coverage   96.47%   96.47%   -0.01%     
==========================================
  Files         469      469              
  Lines       29144    29144              
  Branches     3587     3587              
==========================================
- Hits        28117    28116       -1     
  Misses        840      840              
- Partials      187      188       +1     

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 8, 2023

CodSpeed Performance Report

Merging #3018 will not alter performance

Comparing davidnemec:pydantic-import (ad2dbb8) with main (b456a70)

Summary

✅ 12 untouched benchmarks

Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch! Can you add a release file for this?

@@ -64,6 +63,8 @@
try:
from pydantic.mypy import METADATA_KEY as PYDANTIC_METADATA_KEY
from pydantic.mypy import PydanticModelField

from strawberry.experimental.pydantic._compat import IS_PYDANTIC_V1
except ImportError:
PYDANTIC_METADATA_KEY = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I think it is safe to add a default to false 😊

so

IS_PYDANTIC_V1 = False

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IS_PYDANTIC_V1 is used in if-else. Should I update it to if-elif and also set (and check in elif) IS_PYDANTIC_V2=False? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant only in the expect except ImportError: if we can't import pydantic it means we don't have it installed, so the default for IS_PYDANTIC_V1 doesn't matter that much :)

Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@patrick91 patrick91 merged commit 1b5e39c into strawberry-graphql:main Aug 8, 2023
47 of 51 checks passed
etripier pushed a commit to Greenbax/strawberry that referenced this pull request Oct 25, 2023
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.

3 participants