-
Notifications
You must be signed in to change notification settings - Fork 227
feat(async): add support for async sessions #350
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
Merged
erikwrede
merged 21 commits into
graphql-python:master
from
jendrikjoe:add-async-support
Dec 21, 2022
Merged
Changes from 11 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
32bf439
feat(async): add support for async sessions
41c88f9
fix(test batching): ensure that objects are added to database in asynβ¦
47d224e
test: only run batching tests with sync session
811cdf2
chore(fields): use get_query instead of manually crafting the query
3149830
fix: throw exceptions if Async Session is used with old sql alchemy
0e60c03
test: fix sqlalchemy 1.2 and 1.3 tests, fix batching tests by separatβ¦
0180f69
fix: ensure that synchronous execute calls are still feasible
ec76697
refactor: remove duplicate code by fixing if condition
6a00846
chore: add specific error if awaitable is returned in synchronous exeβ¦
9897a03
chore: merge master, resolve conflicts
fff782f
test: use pytest_asyncio.fixture instead normal fixture, fix issues iβ¦
1250231
chore: remove duplicate eventually_await_session
eee2314
chore: remove duplicate skip statement
bacf15d
fix: fix benchmark tests not being executed properly
2bc6f84
chore: format files
a968ff8
chore: move is_graphene_version_less_than to top of file
1039f03
test: remove unnecessary pytest.mark.asyncio, auto-reformatting
e61df34
chore: revert faulty formatting
2ff54dc
fix: run startup checkt for sqlalchemy version
4321b04
chore: rebase onto master, adapt interface test to run with sync sessβ¦
1e857e0
fix: allow polymorphism with async session
erikwrede File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
from __future__ import absolute_import | ||
|
||
import enum | ||
|
||
from sqlalchemy import ( | ||
Column, | ||
Date, | ||
Enum, | ||
ForeignKey, | ||
Integer, | ||
String, | ||
Table, | ||
func, | ||
select, | ||
) | ||
from sqlalchemy.ext.declarative import declarative_base | ||
from sqlalchemy.orm import column_property, relationship | ||
|
||
PetKind = Enum("cat", "dog", name="pet_kind") | ||
|
||
|
||
class HairKind(enum.Enum): | ||
LONG = "long" | ||
SHORT = "short" | ||
|
||
|
||
Base = declarative_base() | ||
|
||
association_table = Table( | ||
"association", | ||
Base.metadata, | ||
Column("pet_id", Integer, ForeignKey("pets.id")), | ||
Column("reporter_id", Integer, ForeignKey("reporters.id")), | ||
) | ||
|
||
|
||
class Pet(Base): | ||
__tablename__ = "pets" | ||
id = Column(Integer(), primary_key=True) | ||
name = Column(String(30)) | ||
pet_kind = Column(PetKind, nullable=False) | ||
hair_kind = Column(Enum(HairKind, name="hair_kind"), nullable=False) | ||
reporter_id = Column(Integer(), ForeignKey("reporters.id")) | ||
|
||
|
||
class Reporter(Base): | ||
__tablename__ = "reporters" | ||
|
||
id = Column(Integer(), primary_key=True) | ||
first_name = Column(String(30), doc="First name") | ||
last_name = Column(String(30), doc="Last name") | ||
email = Column(String(), doc="Email") | ||
favorite_pet_kind = Column(PetKind) | ||
pets = relationship( | ||
"Pet", | ||
secondary=association_table, | ||
backref="reporters", | ||
order_by="Pet.id", | ||
) | ||
articles = relationship("Article", backref="reporter") | ||
favorite_article = relationship("Article", uselist=False) | ||
|
||
column_prop = column_property( | ||
select([func.cast(func.count(id), Integer)]), doc="Column property" | ||
) | ||
|
||
|
||
class Article(Base): | ||
__tablename__ = "articles" | ||
id = Column(Integer(), primary_key=True) | ||
headline = Column(String(100)) | ||
pub_date = Column(Date()) | ||
reporter_id = Column(Integer(), ForeignKey("reporters.id")) | ||
readers = relationship( | ||
"Reader", secondary="articles_readers", back_populates="articles" | ||
) | ||
|
||
|
||
class Reader(Base): | ||
__tablename__ = "readers" | ||
id = Column(Integer(), primary_key=True) | ||
name = Column(String(100)) | ||
articles = relationship( | ||
"Article", secondary="articles_readers", back_populates="readers" | ||
) | ||
|
||
|
||
class ArticleReader(Base): | ||
__tablename__ = "articles_readers" | ||
article_id = Column(Integer(), ForeignKey("articles.id"), primary_key=True) | ||
reader_id = Column(Integer(), ForeignKey("readers.id"), primary_key=True) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I don't think we need to check this every request, it will just introduce inefficiencies in every request. Maybe a "startup check" or documentation does the job here.
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.
Maybe changing lines 16/17 so that AsyncSession has a defined value (
None
), if sqlalchemy_version < 1.4 will be more elegantThere 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.
The problem here is the
isinstance
check. The import of it only happens if the version is nor less than 1.4. So in all other versions I need to fail the if statement before doing theisinstance
check. I can imagine to ways around:SQLALCHEMY_LESS_THAN_1_4
, so I just need to check the boolean value of the constantI would prefer the first option, as I am not sure how to check if a session is sync in sqlalchemy. There seems to be a protected class attribute
_is_asyncio
with sessions, but checking that would probably agin require checking the version of sqlalchemy first as I assume the property is new π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.
Startup check option sounds great!
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.
We'll need this anyways as 2.0 is in beta; so we should use 1.4 style on all queries if possible - even in sync session. Scope of a different PR though. This will also require the global option.
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.
Implemented the startup option π