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

Graphene v3 (tests) #317

Merged
merged 33 commits into from
Sep 21, 2021
Merged

Graphene v3 (tests) #317

merged 33 commits into from
Sep 21, 2021

Conversation

richin13
Copy link
Contributor

@richin13 richin13 commented Sep 10, 2021

This PR builds on top of #306 and fixes all failing tests. It replaces the promises.dataloader with aiodataloader making batched queries asyncio-dependant. The tests were update to account for this change.

I had to pin sqlalchemy to <1.4 because QueryContext changed its API in 1.4, happy to revisit this decision after pushing the v3 release to unblock graphene's road to v3

Thanks to @colelin26 for adding support for SQLAlchemy 1.4; the above is no longer true.

@richin13 richin13 marked this pull request as ready for review September 10, 2021 16:37
@richin13 richin13 marked this pull request as draft September 10, 2021 18:57
@richin13 richin13 marked this pull request as ready for review September 10, 2021 22:02
@richin13
Copy link
Contributor Author

Fix #248

DoctorJohn and others added 21 commits September 17, 2021 12:22
The same has been done in graphene.
Python 3 bundles mock in stdlib.
Allow for some flexibility on version numbers.
As we're targeting Python >= 3.6, this module should always be in
stdlib.
SQLAlchemy 1.4+ does not export Binary directly. We should decide
whether to keep this test, update for another unknown type, or remove it
altogether.
ChoiceType no longer relies on EnumMeta but instead it exposes a
``type_impl`` field that we can leverage to decide whether the internal
``choices`` is a tuple of enum members or a tuple of tuples (pairs)
@colelin26
Copy link
Contributor

Thanks for the great work! Inspired by the existing work, I gave it a try to support sqlalchemy1.4 and passed all test cases https://github.com/richin13/graphene-sqlalchemy/pull/2. I constructed QueryContext differently and used _load_for_path with another signature when the sqlalchemy version is 1.4. Not sure if this is the best way to solve API change issues, but please take a look if anyone is interested!

@richin13
Copy link
Contributor Author

Hey @mvanlonden 👋🏼 can you help us get this reviewed?

@codecov-commenter
Copy link

Codecov Report

Merging #317 (64d8bb8) into master (cba727c) will decrease coverage by 0.73%.
The diff coverage is 91.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
- Coverage   97.65%   96.91%   -0.74%     
==========================================
  Files           9        9              
  Lines         596      616      +20     
==========================================
+ Hits          582      597      +15     
- Misses         14       19       +5     
Impacted Files Coverage Δ
graphene_sqlalchemy/types.py 94.30% <ø> (ø)
graphene_sqlalchemy/converter.py 96.94% <77.77%> (-1.50%) ⬇️
graphene_sqlalchemy/batching.py 93.33% <86.66%> (-6.67%) ⬇️
graphene_sqlalchemy/fields.py 99.02% <96.42%> (-0.98%) ⬇️
graphene_sqlalchemy/__init__.py 100.00% <100.00%> (ø)
graphene_sqlalchemy/enums.py 97.80% <100.00%> (-0.03%) ⬇️
graphene_sqlalchemy/registry.py 100.00% <100.00%> (ø)
graphene_sqlalchemy/utils.py 95.65% <100.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cba727c...64d8bb8. Read the comment docs.

@mvanlonden mvanlonden merged commit d6dd67e into graphql-python:master Sep 21, 2021
@richin13 richin13 deleted the graphene-v3 branch September 21, 2021 13:50
@zsiciarz zsiciarz mentioned this pull request Sep 21, 2021
@yilinjuang
Copy link

@richin13 thanks for adding graphene v3 support. Is it possible to backport sqlalchemy 1.4 support to latest stable version 2.3? I'm using sqlalchemy 1.4 and really want to test the batching ability before graphene v3 is GA. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants