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

Fix mypy issue with async resolvers #3516

Merged
merged 17 commits into from
May 27, 2024
Merged

Fix mypy issue with async resolvers #3516

merged 17 commits into from
May 27, 2024

Conversation

patrick91
Copy link
Member

@patrick91 patrick91 commented May 27, 2024

From discord, looks like we introduced a regression for mypy in #3241

Not sure why our test suite was passing though, I think before merging we should figure that out (assuming this fixes it properly)

I've updated how we test mypy to prevent similar issue in future

Copy link

codecov bot commented May 27, 2024

Codecov Report

Attention: Patch coverage is 63.72549% with 111 lines in your changes are missing coverage. Please review.

Project coverage is 94.34%. Comparing base (a7fc70a) to head (c4056c1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3516      +/-   ##
==========================================
- Coverage   96.53%   94.34%   -2.19%     
==========================================
  Files         520      519       -1     
  Lines       33243    32183    -1060     
  Branches     5527     3683    -1844     
==========================================
- Hits        32090    30363    -1727     
- Misses        920     1538     +618     
- Partials      233      282      +49     

Copy link

codspeed-hq bot commented May 27, 2024

CodSpeed Performance Report

Merging #3516 will not alter performance

Comparing fix/mypy-async (c4056c1) with main (a7fc70a)

Summary

✅ 12 untouched benchmarks

@botberry
Copy link
Member

botberry commented May 27, 2024

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🔲
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

@patrick91 patrick91 marked this pull request as ready for review May 27, 2024 22:52
@botberry
Copy link
Member

botberry commented May 27, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release fixes an issue where mypy would complain when using a typed async
resolver with strawberry.field(resolver=...).

Now the code will type check correctly. We also updated our test suite to make
we catch similar issues in the future.

Here's the tweet text:

🆕 Release (next) is out! This release fixes an issue that caused type
checking errors when using Mypy and async resolvers.

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: 6 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 10 issues found
  • 🟢 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/field.py Show resolved Hide resolved
strawberry/field.py Show resolved Hide resolved
strawberry/field.py Show resolved Hide resolved
strawberry/field.py Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
tests/typecheckers/test_fields.py Show resolved Hide resolved
tests/typecheckers/test_fields_input.py Show resolved Hide resolved
tests/typecheckers/test_union.py Show resolved Hide resolved
tests/typecheckers/test_auto.py Show resolved Hide resolved
tests/typecheckers/utils/mypy.py Show resolved Hide resolved
@patrick91 patrick91 changed the title Attempt at fixing mypy error with async resolvers Fix mypy issue with async resolvers May 27, 2024
@patrick91 patrick91 merged commit cab16ee into main May 27, 2024
63 of 65 checks passed
@patrick91 patrick91 deleted the fix/mypy-async branch May 27, 2024 23:20
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.

2 participants