strawberry-sqlalchemy icon indicating copy to clipboard operation
strawberry-sqlalchemy copied to clipboard

update imports for strawberry 0.236.0

Open novag opened this issue 1 year ago β€’ 12 comments

Description

strawberry 0.236.0 changed the code structure which requires adjusting the imports.

Types of Changes

  • [ ] Core
  • [x] Bugfix
  • [ ] New feature
  • [ ] Enhancement/optimization
  • [ ] Documentation

Issues Fixed or Closed by This PR

Checklist

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

Summary by Sourcery

This pull request updates the import paths in the relay.py file to be compatible with the changes in the code structure of strawberry 0.236.0, ensuring the project remains functional with the latest version.

  • Bug Fixes:
    • Updated import paths to align with the new code structure introduced in strawberry 0.236.0.

novag avatar Jul 20 '24 18:07 novag

Reviewer's Guide by Sourcery

This pull request updates the import statements in 'src/strawberry_sqlalchemy_mapper/relay.py' to be compatible with the new code structure introduced in strawberry 0.236.0. Specifically, the imports for 'StrawberryContainer' and 'get_object_definition' were changed from 'strawberry.type' to 'strawberry.types.base'.

File-Level Changes

Files Changes
src/strawberry_sqlalchemy_mapper/relay.py Adjusted imports to align with the new code structure introduced in strawberry 0.236.0.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

sourcery-ai[bot] avatar Jul 20 '24 18:07 sourcery-ai[bot]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Updated imports to be compatible with strawberry 0.236.0 Increased the minimum required strawberry version to 0.236.0

botberry avatar Jul 20 '24 18:07 botberry

CodSpeed Performance Report

Merging #187 will not alter performance

Comparing novag:main (e124a16) with main (3397507)

Summary

βœ… 1 untouched benchmarks

codspeed-hq[bot] avatar Jul 21 '24 11:07 codspeed-hq[bot]

I'll check this this week 😊 not sure what happened with the CI :D

patrick91 avatar Jul 25 '24 09:07 patrick91

Any update here? It would be great to merge this in to unblock strawberry updates. Happy to contribute somehow.

bperkins24 avatar Aug 05 '24 19:08 bperkins24

Hi, @novag Today I joined the Strawberry team as the maintainer for the SQLAlchemy integration. I’ve already started reviewing your code and investigating the issues with our CI. Thank you for your contributions!

Ckk3 avatar Sep 26 '24 00:09 Ckk3

Hi, @novag , we have fixed the CI, can you please update your branch from main? Also, I think we missing the new strawberry version inside the poetry.lock file, can you update this too?

Ckk3 avatar Oct 02 '24 13:10 Ckk3

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.38%. Comparing base (3397507) to head (e124a16).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #187   +/-   ##
=======================================
  Coverage   85.38%   85.38%           
=======================================
  Files          16       16           
  Lines        1615     1615           
  Branches      128      128           
=======================================
  Hits         1379     1379           
  Misses        183      183           
  Partials       53       53           

codecov-commenter avatar Oct 02 '24 14:10 codecov-commenter

Thanks, @novag , please fix the failed tests when you can πŸ˜‰

Ckk3 avatar Oct 02 '24 14:10 Ckk3

Just to make your job a little more fast, I've checked that you cax fix by just changing _fields to fields (since StrawberryObjectDefinition has been changed) image

I've tested this locally and worked on both failing tests

Ckk3 avatar Oct 02 '24 14:10 Ckk3

Yes, thanks. I'm trying to determine what has changed that's now preventing the linter from inferring the lambda type.

novag avatar Oct 02 '24 15:10 novag

Hi @novag, I hope the investigation is going well! It’s been a week since we last discussed it. What do you think about pushing the current changes, especially to the tests, to see if the link tests are still failing? Let me know if you need any help!

Ckk3 avatar Oct 10 '24 15:10 Ckk3

@novag I noticed that the CI failed again on the lint check, but there's also a new error that has surfaced. It seems related to the new Ubuntu LTS (benchmark) and the Python environment (test matrix generation). I'll investigate further on another PR.

Ckk3 avatar Oct 10 '24 23:10 Ckk3

Hi, @novag , the CI fix is already on main, please update your branch when you can.

Ckk3 avatar Oct 13 '24 15:10 Ckk3

@Ckk3 Thanks for fixing the CI! I am currently very limited in time, however I will have another look at the linter error today or tomorrow.

novag avatar Oct 14 '24 09:10 novag

Hi @novag. It would be great to get this done. There are features in newer versions of strawberry that are very useful for my firm and we are blocked on using them until this is resolved.

bperkins24 avatar Oct 17 '24 15:10 bperkins24

mypy fails at line 213 in src/strawberry_sqlalchemy_mapper/field.py because it cannot infer the type of the lambda expression. Although I don't think there's anything wrong here, I have not yet been able to find out why this check would have been successful before updating to strawberry 0.236.0. mypy should be able to infer the type from the default arguments given to the lambda expression. field.py is also sprinkled with # type: ignore comments. If that's finde for @Ckk3 we can just add one after the lambda expression.

If anyone is more familiar with mypy, I would appreciate any input.

novag avatar Oct 17 '24 16:10 novag

I like this option, what you guys think @patrick91 @bellini666 @TimDumol ?

Ckk3 avatar Oct 17 '24 16:10 Ckk3

I like this option, what you guys think @patrick91 @bellini666 @TimDumol ?

I think it is fine to ignore that specific error, even more because it is not actually related to this PR :)

bellini666 avatar Oct 17 '24 16:10 bellini666

LGTM, let's just wait to see what the other maintainers think.

Ckk3 avatar Oct 17 '24 19:10 Ckk3

Thanks for contributing to Strawberry! πŸŽ‰ You've been invited to join the Strawberry GraphQL organisation 😊

You can also request a free sticker by filling this form: https://forms.gle/dmnfQUPoY5gZbVT67

And don't forget to join our discord server: https://strawberry.rocks/discord πŸ”₯

botberry avatar Oct 17 '24 21:10 botberry

Thanks @novag !

Ckk3 avatar Oct 17 '24 21:10 Ckk3