peterschutt
commented
1 year ago Thanks for creating these issues @AgarwalPragy - very helpful. I'll do my best to take a look at them over the next day or so.
Closed AgarwalPragy closed 1 year ago
peterschutt
commented
1 year ago Thanks for creating these issues @AgarwalPragy - very helpful. I'll do my best to take a look at them over the next day or so.
cofin
commented
1 year ago @AgarwalPragy Can you confirm that this is all of the code?
I am able to reproduce the behavior you describe. However, this example does not work outside of Litestar either. Here's an example that uses only SQLAlchemy and shows the same exception.
from __future__ import annotations
from datetime import datetime
from typing import TYPE_CHECKING, Any, ClassVar
from uuid import UUID
import anyio
from sqlalchemy import BIGINT, TIMESTAMP, Boolean, Index, MetaData, String, Uuid, func, select
from sqlalchemy.ext.asyncio import AsyncAttrs, AsyncSession, async_sessionmaker, create_async_engine
from sqlalchemy.orm import (
DeclarativeBase,
Mapped,
declared_attr,
mapped_column,
registry,
relationship,
)
if TYPE_CHECKING:
from sqlalchemy.types import TypeEngine
db_url = "sqlite+aiosqlite:///:memory:"
meta = MetaData()
type_annotation_map: dict[type, type[TypeEngine[Any]] | TypeEngine[Any]] = {
int: BIGINT,
datetime: TIMESTAMP(timezone=True),
str: String,
UUID: Uuid,
bool: Boolean,
}
orm_registry = registry(metadata=meta, type_annotation_map=type_annotation_map)
engine = create_async_engine(db_url)
async_session_factory: async_sessionmaker[AsyncSession] = async_sessionmaker(engine, expire_on_commit=False)
class ModelBase(AsyncAttrs, DeclarativeBase):
__abstract__ = True
__name__: ClassVar[str]
registry = orm_registry
id: Mapped[int] = mapped_column(primary_key=True)
created_at: Mapped[datetime] = mapped_column(index=True, doc="Time of creation", server_default=func.now())
updated_at: Mapped[datetime] = mapped_column(
index=True,
doc="Time of last modification",
server_default=func.now(),
onupdate=func.now(),
)
# noinspection PyMethodParameters
@declared_attr.directive
def __tablename__(cls) -> str:
"""Automatically generate table name from class name."""
return f"{cls.__name__.lower()}s"
def to_dict(self, exclude: set[str] | None = None) -> dict[str, Any]:
"""Convert model to dictionary.
Returns:
dict[str, Any]: A dict representation of the model
"""
exclude = {"sa_orm_sentinel", "_sentinel"}.union(self._sa_instance_state.unloaded).union(exclude or []) # type: ignore[attr-defined]
return {field.name: getattr(self, field.name) for field in self.__table__.columns if field.name not in exclude}
class GenericItemRevision:
target_id: Mapped[UUID] = mapped_column(index=True, nullable=False)
data: Mapped[str]
class RevisionsMixin:
@declared_attr.directive
def revisions(cls) -> Mapped[list[GenericItemRevision]]:
revision_class_name = f"{cls.__name__}Revision"
revision_table_name = f"{cls.__name__.lower()}_revisions"
class Revision(ModelBase, GenericItemRevision):
__tablename__ = revision_table_name # type: ignore
__table_args__ = (Index(f"ix_{revision_table_name}_target_id_created_at", "target_id", "created_at"),)
Revision.__name__ = revision_class_name
return relationship(
revision_class_name,
order_by=f"desc({revision_table_name}.created_at)",
back_populates="target",
)
class Note(ModelBase, RevisionsMixin):
contents: Mapped[str]
async def build_and_query() -> None:
async with engine.begin() as conn:
await conn.run_sync(meta.create_all)
async with async_session_factory() as session:
results = await session.execute(select(Note))
print(results)
if __name__ == "__main__":
anyio.run(build_and_query)Also, you can simplify your dependency injection by using the built in plugin's dependency injection. You can configure it to inject a session with the transaction parameter like so:
@get("/")
async def hello(transaction: AsyncSession) -> Sequence[Note]:
query = select(Note)
result = await transaction.execute(query)
return result.scalars().all()
db_url = "sqlite+aiosqlite:///:memory:"
app = Litestar(
route_handlers=[hello],
plugins=[
SQLAlchemyPlugin(
config=SQLAlchemyAsyncConfig(
connection_string=db_url,
session_dependency_key="transaction",
create_all=True,
alembic_config=AlembicAsyncConfig(target_metadata=orm_registry.metadata),
),
),
],
)
Also, you can simplify your dependency injection by using the built in plugin's dependency injection. You can configure it to inject a session with the transaction parameter like so:
The transaction dependency in OP is additional the one in the plugin - it actually depends on the plugin provided one - and opens a transaction, with handling for some dbapi errors. Its copying a pattern from the tutorial that extends the TODO app with sqlalchemy - which was written before the plugin had any auto-committing behavior.
AgarwalPragy
commented
1 year ago @cofin interesting. Since the alembic migrations were working, I assumed that my code was correct. Seems like the migrations work, but the queries don't.
My bad. Closing this issue :)
fixed my code, and it works now 🎉
AgarwalPragy
commented
1 year ago db_url = "sqlite+aiosqlite:///:memory:" app = Litestar( route_handlers=[hello], plugins=[ SQLAlchemyPlugin( config=SQLAlchemyAsyncConfig( connection_string=db_url, session_dependency_key="transaction", create_all=True, alembic_config=AlembicAsyncConfig(target_metadata=orm_registry.metadata), ), ), ], )
With litestar==2.2.1, I get TypeError: SQLAlchemyAsyncConfig.__init__() got an unexpected keyword argument 'create_all'
peterschutt
commented
1 year ago make sure you have latest advanced-alchemy package - this parameter was only just added.
AgarwalPragy
commented
1 year ago db_url = "sqlite+aiosqlite:///:memory:" app = Litestar( route_handlers=[hello], plugins=[ SQLAlchemyPlugin( config=SQLAlchemyAsyncConfig( connection_string=db_url, session_dependency_key="transaction", create_all=True, alembic_config=AlembicAsyncConfig(target_metadata=orm_registry.metadata), ), ), ], )
@cofin this might be due to my lack of understanding, but adding session_dependency_key="transaction" prevents any data from being saved to the db. With this line, my POST endpoints return the data, but nothing is saved in db. Commenting this line make them work as expected!
cofin
commented
1 year ago @AgarwalPragy You are correct. By default, the plugin session handler does not automatically commit on a successful response. You can easily change it by using the following before_send handler:
from advanced_alchemy.extensions.litestar.plugins.init.config.asyncio import autocommit_before_send_handler
db_url = "sqlite+aiosqlite:///:memory:"
app = Litestar(
route_handlers=[hello],
plugins=[
SQLAlchemyPlugin(
config=SQLAlchemyAsyncConfig(
connection_string=db_url,
session_dependency_key="transaction",
create_all=True,
alembic_config=AlembicAsyncConfig(target_metadata=orm_registry.metadata),
before_send_handler=autocommit_before_send_handler,
),
),
],
)
@AgarwalPragy You are correct. By default, the plugin session handler does not automatically commit on a successful response. You can easily change it by using the following
before_sendhandler:from advanced_alchemy.extensions.litestar.plugins.init.config.asyncio import autocommit_before_send_handler db_url = "sqlite+aiosqlite:///:memory:" app = Litestar( route_handlers=[hello], plugins=[ SQLAlchemyPlugin( config=SQLAlchemyAsyncConfig( connection_string=db_url, session_dependency_key="transaction", create_all=True, alembic_config=AlembicAsyncConfig(target_metadata=orm_registry.metadata), before_send_handler=autocommit_before_send_handler, ), ), ], )
I'd say this is a documentation bug on our side now.
We should update https://docs.litestar.dev/latest/tutorials/sqlalchemy/3-init-plugin.html to do the same as this (it was written before the autocommit handler was a part of the plugin, IIRC).
Description
I'm porting my app from FastAPI to Litestar.
Models which worked in FastAPI using SQLAlchemy (async) + Alembic no longer work in Litestar
It throws the following error
MCVE
Comparision b/w Alembic and Litestar https://github.com/AgarwalPragy/litestar-bug-report
Sample code for Litestar
Litestar Version
litestar==2.2.1 aiosqlite==0.19.0 alembic==1.12.1 SQLAlchemy==2.0.22
Platform
Logs