feat(async): add support for async sessions

[jendrikjoe]

This is my initial suggestion on how to support async sessions in graphene-sqlalchemy as discussed in https://github.com/graphql-python/graphene-sqlalchemy/issues/324 🙂 It is not completely done yet, but I thought I would already open it to give you a chance for some feedback. Sorry as well for all the style changes. My IDE ran black over it. I can revert those changes, however I couldn't find anything in the contribution guidelines regarding the style, so I thought I keep it for the time being 🤷‍♂️ As mentioned in the original issue: Async only supports joined and selectin loads and no lazy ones. This should be stated somewhere in the docs, I assume 😉

Open points:

• [ ] Add async tests for batching (do we need this? When reading this comment: https://github.com/graphql-python/graphene-sqlalchemy/issues/35#issuecomment-1107463349, batching seems to be used with lazy connections which anyhow don't work with async 🤔 )
• [ ] Add limitation on relationships to docs

[erikwrede]

Thank you! I'll have a look at this in the next days.

[codecov-commenter]

Codecov Report

Base: 97.09% // Head: 96.24% // Decreases project coverage by -0.84% :warning:

Coverage data is based on head (1e857e0) compared to base (2edeae9). Patch coverage: 90.74% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #350      +/-   ##
==========================================
- Coverage   97.09%   96.24%   -0.85%     
==========================================
  Files           7        9       +2     
  Lines         722      879     +157     
==========================================
+ Hits          701      846     +145     
- Misses         21       33      +12     

Impacted Files	Coverage Δ
graphene_sqlalchemy/batching.py	95.34% <75.00%> (ø)
graphene_sqlalchemy/types.py	93.45% <76.47%> (-1.95%)	:arrow_down:
graphene_sqlalchemy/fields.py	98.58% <100.00%> (-0.58%)	:arrow_down:
graphene_sqlalchemy/utils.py	93.85% <100.00%> (ø)
graphene_sqlalchemy/__init__.py	100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[erikwrede]

Looks like everything is coming together, I'll check the new changes out soon! Thank you for all the effort 🙂

@Fred7b do you also have time for a full review? If I recall correctly, u originally created the issue so I would appreciate the input from you! Formatting will be solved by black soon, so no need to care for that on the review.

[jendrikjoe]

A polite ping 🙂

[erikwrede]

@jendrikjoe sorry, I've been focused on other issues. My plan is to merge those first, release a new beta with the other issues and then immediately release another beta including async session support so both versions can be tried out, in case async session support still has some bugs we didn't find. If there's anything left to change I'll let you know in the next days :)

[jendrikjoe]

@erikwrede no worries and thanks for explaining 🤗 The release plan makes a lot of sense and take all the time you need for it 👍 Just wanted to make sure that the MR is not forgotten 😉

[erikwrede]

Quick Update: Release of this is pushed back until #355 is merged. We're reformatting connection fields and it makes sense to merge that first and then add async sessions later. Should require minor reformatting here once #355 is done. I definitely want to get this integrated afterwards. Sorry to keep you waiting, thanks for all the patience! 🙂

[jendrikjoe]

No worries @erikwrede 🤗 Just ping me whenever I should resolve conflicts here 👍

[erikwrede]

Hey @jendrikjoe Sorry it took so long! The consolidation is done and ready to be addressed. Should only be causing minor conflicts. LMK if you need any help 🙂

[jendrikjoe]

Thanks for pinging me 👍 Will try resolve the conflicts later today 🙂

[erikwrede]

Great!

I noticed that some of the original tests are still modified to use async methods. I'd prefer that entire part to be separate - leave the sync tests and Schema resolvers as is and create separate object types with async resolvers and corresponding tests. Otherwise a test failure will not tell wether the failure hints at a fundamental problem or problems with the async extension. What do you think?

[jendrikjoe]

I parametrize the schema now to have a synchronous and an asynchronous for all relevant tests. This way every test reports with which schema it failed, while still avoiding to copy all the tests. Or are you referring to the changes I made on the lazy attributes of the SQLModels?

I still have to investigate why the batching tests using get_full_relay_schema are failing for me. Presumably, because I separated the batching models from the others, so the lazy attribute is not set on them.

I as well moved some assert statements in the batching tests. I had some bugs in them, which led to the result of querying the schema having an error and then of course wrong sql statement counts, too. However, first testing that no errors occur when calling the schema, before counting the SQL statements made the debugging a lot easier and the error more apparent.

[jendrikjoe]

Will try to figure as well what the issues with the session fixture is 🙈

[jendrikjoe]

Should be all working now 🥳