-
Notifications
You must be signed in to change notification settings - Fork 228
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
feat: SQLAlchemy 2.0 support #368
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #368 +/- ##
==========================================
- Coverage 96.39% 96.39% -0.01%
==========================================
Files 9 9
Lines 916 915 -1
==========================================
- Hits 883 882 -1
Misses 33 33
☔ View full report in Codecov by Sentry. |
Signed-off-by: Erik Wrede <[email protected]>
Signed-off-by: Erik Wrede <[email protected]>
Signed-off-by: Erik Wrede <[email protected]>
Some Exception ignored in: <function _ConnectionRecord.checkout.<locals>.<lambda> at 0x11159d7a0>
Traceback (most recent call last):
File "/Users/erik/dev/graphene-sqlalchemy/venv/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 734, in <lambda>
if _finalize_fairy is not None
File "/Users/erik/dev/graphene-sqlalchemy/venv/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 1030, in _finalize_fairy
fairy.dbapi_connection = fairy._connection_record = None # type: ignore
AttributeError: 'NoneType' object has no attribute 'dbapi_connection' These seem to be related to the pytest fixtures, not the actual test cases. I haven't been able to identify a cause so far. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest seems great to me 👍
not SQL_VERSION_HIGHER_EQUAL_THAN_1_4, | ||
reason="SQLAlchemy <1.4 does not support this", | ||
) | ||
def test_should_columproperty_convert_sqa_20(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: You could hand the select function and parametrize the test depending on the SQL version 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good Idea!
@erikwrede just checking in to see if there's any help you need here to get this to the finish line? I'm not sure if this is paused because of some other piece of dependent work, but I'm very excited to use |
@pappasam on this branch, all tests are passing on 2.0. Fixing the review comments is still on my list and will be done soon. You can already start testing by installing this branch via pip. Async sessions are also supported already. Would appreciate any feedback! :) |
@erikwrede I can confirm that this branch works for me! No issues I can find on my end; I'm very much looking forward to this making it into a beta release |
How's it going? I use sqlalchemy 2 and it would be nice to have an official version of this. FWIW, with this branch at least the example in the readme works. In pipenv it can be installed with:
|
Awesome, thank you! |
SQLAlchemy 2.0 is coming soon. This PR is going to collect all of the necessary changes to support it.