✨ Add implementation of AsyncSession.run_sync()#1347
✨ Add implementation of AsyncSession.run_sync()#1347jnewbery wants to merge 2 commits intofastapi:mainfrom
AsyncSession.run_sync()#1347Conversation
svlandeg
changed the title
svlandeg
changed the title
AsyncSession.run_sync()
|
Thanks for the contribution! Could you look into the failing tests? I'll mark this PR as "in draft" in the meantime 🙏 |
jnewbery
force-pushed
the
run_sync
branch
2 times, most recently
from
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.
Thanks @svlandeg . I've fixed the failing tests on old Python versions (by importing |
|
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
left a comment
YuriiMotov
left a comment
There was a problem hiding this comment.
@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).
Would you like to continue working on this?
| async def run_sync( | ||
| self, | ||
| fn: Callable[Concatenate[Session, _P], _TSelectParam], | ||
| *arg: _P.args, | ||
| **kw: _P.kwargs, | ||
| ) -> _TSelectParam: | ||
| base_fn = cast(Callable[Concatenate[_Session, _P], _TSelectParam], fn) |
There was a problem hiding this comment.
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. |
|
As this PR has been waiting for the original user for a while but seems to be inactive, it's now going to be closed. But if there's anyone interested, feel free to create a new PR. |
4 participants
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.