-
-
Notifications
You must be signed in to change notification settings - Fork 782
✨ Add implementation of AsyncSession.run_sync()
#1347
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
Conversation
AsyncSession.run_sync() (追記ここまで)
svlandeg
commented
Apr 28, 2025
Thanks for the contribution! Could you look into the failing tests? I'll mark this PR as "in draft" in the meantime 🙏
cca0285 to
674d759
Compare
Currently, `sqlmodel.ext.asyncio.session.AsyncSession` doesn't implement `run_sync()`, which means that any call to `run_sync()` on a sqlmodel `AsyncSession` will be dispatched to the parent `sqlalchemy.ext.asyncio.AsyncSession`. The first argument to sqlalchemy's `AsyncSession.run_sync()` is a callable whose first argument is a `sqlalchemy.orm.Session` object. If we're using this in a repo that uses sqlmodel, we'll actually be passing a callable whose first argument is a `sqlmodel.orm.session.Session`. In practice this works fine - because `sqlmodel.orm.session.Session` is derived from `sqlalchemy.orm.Session`, the implementation of `sqlalchemy.ext.asyncio.AsyncSession.run_sync()` can use the sqlmodel `Session` object in place of the sqlalchemy `Session` object. However, static analysers will complain that the argument to `run_sync()` is of the wrong type. For example, here's a warning from pyright: ``` Pyright: Error: Argument of type "(session: Session, id: UUID) -> int" cannot be assigned to parameter "fn" of type "(Session, **_P@run_sync) -> _T@run_sync" in function "run_sync" Type "(session: Session, id: UUID) -> int" is not assignable to type "(Session, id: UUID) -> int" Parameter 1: type "Session" is incompatible with type "Session" "sqlalchemy.orm.session.Session" is not assignable to "sqlmodel.orm.session.Session" [reportArgumentType] ``` This commit implements a `run_sync()` method on `sqlmodel.ext.asyncio.session.AsyncSession`, which casts the callable to the correct type before dispatching it to the base class. This satisfies the static type checks.
674d759 to
7c5bdc9
Compare
jnewbery
commented
May 6, 2025
Thanks for the contribution! Could you look into the failing tests? I'll mark this PR as "in draft" in the meantime 🙏
Thanks @svlandeg . I've fixed the failing tests on old Python versions (by importing Concatenate and ParamSpec from typing_extensions instead of typing). I think this is now ready for review.
jnewbery
commented
Jun 3, 2025
Hi @svlandeg . Is there anything I can do to help this get merged? This change is really helpful for us to be able to upgrade to sqlalchemy2.
@YuriiMotov
YuriiMotov
left a comment
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.
@jnewbery, thanks for your interest and efforts!
I think we should take more complex approach to this. For now, if you try creating a test for this method, you will still have import some classes from sqlalchemy, and you will still have error from IDE or static type checkers that run_sync method doesn't exists:
from sqlalchemy.ext.asyncio import AsyncEngine from sqlmodel import Field, Session, SQLModel from sqlmodel.ext.asyncio.session import AsyncSession class MyObject(SQLModel, table=True): id: int = Field(primary_key=True) param: str def some_business_method(session: Session, param: str) -> str: session.add(MyObject(param=param)) session.flush() return "success" async def do_something_async(async_engine: AsyncEngine) -> None: with AsyncSession(async_engine) as async_session: return_code = await async_session.run_sync( some_business_method, param="param1" ) print(return_code)
(code example is taken from the type hint IDE gives when you hover over AsyncSession.run_sync() - this is actually the docstring inherited from sqlalchemy's AsyncSession).
image
Would you like to continue working on this?
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.
The docstring should be updated. Now IDE shows the docstring from sqlalchemy's method
Heads-up: this will be closed in 3 days unless there’s new activity.
Currently,
sqlmodel.ext.asyncio.session.AsyncSessiondoesn't implementrun_sync(), which means that any call torun_sync()on a sqlmodelAsyncSessionwill be dispatched to the parentsqlalchemy.ext.asyncio.AsyncSession.The first argument to sqlalchemy's
AsyncSession.run_sync()is a callable whose first argument is asqlalchemy.orm.Sessionobject. If we're using this in a repo that uses sqlmodel, we'll actually be passing a callable whose first argument is asqlmodel.orm.session.Session.In practice this works fine - because
sqlmodel.orm.session.Sessionis derived fromsqlalchemy.orm.Session, the implementation ofsqlalchemy.ext.asyncio.AsyncSession.run_sync()can use the sqlmodelSessionobject in place of the sqlalchemySessionobject. However, static analysers will complain that the argument torun_sync()is of the wrong type. For example, here's a warning from pyright:This commit implements a
run_sync()method onsqlmodel.ext.asyncio.session.AsyncSession, which casts the callable to the correct type before dispatching it to the base class. This satisfies the static type checks.