Closed richin13 closed 3 years ago
Fix #248
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!
Hey @mvanlonden 👋🏼 can you help us get this reviewed?
Merging #317 (64d8bb8) into master (cba727c) will decrease coverage by
0.73%
. The diff coverage is91.37%
.
@@ 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%) |
:arrow_down: |
graphene_sqlalchemy/batching.py | 93.33% <86.66%> (-6.67%) |
:arrow_down: |
graphene_sqlalchemy/fields.py | 99.02% <96.42%> (-0.98%) |
:arrow_down: |
graphene_sqlalchemy/__init__.py | 100.00% <100.00%> (ø) |
|
graphene_sqlalchemy/enums.py | 97.80% <100.00%> (-0.03%) |
:arrow_down: |
graphene_sqlalchemy/registry.py | 100.00% <100.00%> (ø) |
|
graphene_sqlalchemy/utils.py | 95.65% <100.00%> (+0.19%) |
:arrow_up: |
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.
@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!
This PR builds on top of #306 and fixes all failing tests. It replaces the
promises.dataloader
withaiodataloader
making batched queries asyncio-dependant. The tests were update to account for this change.~I had to pin sqlalchemy to
<1.4
becauseQueryContext
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.