-
-
Notifications
You must be signed in to change notification settings - Fork 782
update async extension and add tests #635
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| from .engine import create_async_engine as create_async_engine | ||
| from .session import AsyncSession as AsyncSession |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| from typing import Any | ||
|
|
||
| from sqlalchemy.ext.asyncio import AsyncEngine | ||
| from sqlalchemy.ext.asyncio import create_async_engine as _create_async_engine | ||
|
|
||
|
|
||
| # create_async_engine by default already has future set to be true. | ||
| # Porting this over to sqlmodel to make it easier to use. | ||
| def create_async_engine(*args: Any, **kwargs: Any) -> AsyncEngine: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This way we don't have autocompletion and type hints for parameters |
||
| return _create_async_engine(*args, **kwargs) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import asyncio | ||
| from typing import Generator, Optional | ||
|
|
||
| import pytest | ||
| from sqlmodel import Field, SQLModel, select | ||
| from sqlmodel.ext.asyncio import AsyncSession, create_async_engine | ||
| from testcontainers.postgres import PostgresContainer | ||
|
|
||
|
|
||
| # The first time this test is run, it will download the postgres image which can take | ||
| # a while. Subsequent runs will be much faster. | ||
| @pytest.fixture(scope="module") | ||
| def postgres_container_url() -> Generator[str, None, None]: | ||
| with PostgresContainer("postgres:13") as postgres: | ||
| postgres.driver = "asyncpg" | ||
| yield postgres.get_connection_url() | ||
|
|
||
|
|
||
| async def _test_async_create(postgres_container_url: str) -> None: | ||
| class Hero(SQLModel, table=True): | ||
| # SQLModel.metadata is a singleton and the Hero Class has already been defined. | ||
| # If I flush the metadata during this test, it will cause test_enum to fail | ||
| # because in that file, the model isn't defined within a function. For now, the | ||
| # workaround is to set extend_existing to True. In the future, test setup and | ||
| # teardown should be refactored to avoid this issue. | ||
| __table_args__ = {"extend_existing": True} | ||
|
Comment on lines
+21
to
+26
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can clear DB before each test, that's common approach. |
||
|
|
||
| id: Optional[int] = Field(default=None, primary_key=True) | ||
| name: str | ||
| secret_name: str | ||
| age: Optional[int] = None | ||
|
|
||
| hero_create = Hero(name="Deadpond", secret_name="Dive Wilson") | ||
|
|
||
| engine = create_async_engine(postgres_container_url) | ||
| async with engine.begin() as conn: | ||
| await conn.run_sync(SQLModel.metadata.create_all) | ||
|
|
||
| async with AsyncSession(engine) as session: | ||
| session.add(hero_create) | ||
| await session.commit() | ||
| await session.refresh(hero_create) | ||
|
|
||
| async with AsyncSession(engine) as session: | ||
| statement = select(Hero).where(Hero.name == "Deadpond") | ||
| results = await session.exec(statement) | ||
| hero_query = results.one() | ||
| assert hero_create == hero_query | ||
|
|
||
|
|
||
| def test_async_create(postgres_container_url: str) -> None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to use |
||
| asyncio.run(_test_async_create(postgres_container_url)) | ||
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.
It may be good idea in general to introduce testcontainers and test SQLModel with real Postgres, but for the reasons of this PR I think we can just use
aiosqlitedriver and in-memory DB