From f5442e111c0941d62419582237cd368c49e66c5d Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 22:13:46 +0000 Subject: [PATCH 01/16] Support async sessions in strawberry_sqlalchemy_mapper and deprecate sync. I left a note in the loader's __init__, but to duplicate that here: Making blocking database calls from within an async function (the resolver) has catastrophic performance implications. Not only will all resolvers be effectively serialized, any other coroutines waiting on the event loop (e.g. concurrent requests in a web server), will be blocked as well, grinding your entire service to a halt. There's no reason for us to support a foot bazooka, hence the deprecation. --- RELEASE.md | 3 ++ poetry.lock | 57 +++++++++++++++++++++- pyproject.toml | 5 +- src/strawberry_sqlalchemy_mapper/loader.py | 38 +++++++++++++-- tests/conftest.py | 36 +++++++++++++- tests/test_loader.py | 47 +++++++++++++++--- 6 files changed, 171 insertions(+), 15 deletions(-) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000..777dde2 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,3 @@ +Release type: minor + +Adds support for async sessions and deprecates sync sessions due to performance reasons. diff --git a/poetry.lock b/poetry.lock index ede5ac6..6b76d67 100644 --- a/poetry.lock +++ b/poetry.lock @@ -40,6 +40,59 @@ files = [ six = ">=1.6.1,<2.0" wheel = ">=0.23.0,<1.0" +[[package]] +name = "asyncpg" +version = "0.28.0" +description = "An asyncio PostgreSQL driver" +optional = false +python-versions = ">=3.7.0" +files = [ + {file = "asyncpg-0.28.0-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:0a6d1b954d2b296292ddff4e0060f494bb4270d87fb3655dd23c5c6096d16d83"}, + {file = "asyncpg-0.28.0-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:0740f836985fd2bd73dca42c50c6074d1d61376e134d7ad3ad7566c4f79f8184"}, + {file = "asyncpg-0.28.0-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:e907cf620a819fab1737f2dd90c0f185e2a796f139ac7de6aa3212a8af96c050"}, + {file = "asyncpg-0.28.0-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:86b339984d55e8202e0c4b252e9573e26e5afa05617ed02252544f7b3e6de3e9"}, + {file = "asyncpg-0.28.0-cp310-cp310-musllinux_1_1_aarch64.whl", hash = "sha256:0c402745185414e4c204a02daca3d22d732b37359db4d2e705172324e2d94e85"}, + {file = "asyncpg-0.28.0-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:c88eef5e096296626e9688f00ab627231f709d0e7e3fb84bb4413dff81d996d7"}, + {file = "asyncpg-0.28.0-cp310-cp310-win32.whl", hash = "sha256:90a7bae882a9e65a9e448fdad3e090c2609bb4637d2a9c90bfdcebbfc334bf89"}, + {file = "asyncpg-0.28.0-cp310-cp310-win_amd64.whl", hash = "sha256:76aacdcd5e2e9999e83c8fbcb748208b60925cc714a578925adcb446d709016c"}, + {file = "asyncpg-0.28.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:a0e08fe2c9b3618459caaef35979d45f4e4f8d4f79490c9fa3367251366af207"}, + {file = "asyncpg-0.28.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:b24e521f6060ff5d35f761a623b0042c84b9c9b9fb82786aadca95a9cb4a893b"}, + {file = "asyncpg-0.28.0-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:99417210461a41891c4ff301490a8713d1ca99b694fef05dabd7139f9d64bd6c"}, + {file = "asyncpg-0.28.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f029c5adf08c47b10bcdc857001bbef551ae51c57b3110964844a9d79ca0f267"}, + {file = "asyncpg-0.28.0-cp311-cp311-musllinux_1_1_aarch64.whl", hash = "sha256:ad1d6abf6c2f5152f46fff06b0e74f25800ce8ec6c80967f0bc789974de3c652"}, + {file = "asyncpg-0.28.0-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:d7fa81ada2807bc50fea1dc741b26a4e99258825ba55913b0ddbf199a10d69d8"}, + {file = "asyncpg-0.28.0-cp311-cp311-win32.whl", hash = "sha256:f33c5685e97821533df3ada9384e7784bd1e7865d2b22f153f2e4bd4a083e102"}, + {file = "asyncpg-0.28.0-cp311-cp311-win_amd64.whl", hash = "sha256:5e7337c98fb493079d686a4a6965e8bcb059b8e1b8ec42106322fc6c1c889bb0"}, + {file = "asyncpg-0.28.0-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:1c56092465e718a9fdcc726cc3d9dcf3a692e4834031c9a9f871d92a75d20d48"}, + {file = "asyncpg-0.28.0-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:4acd6830a7da0eb4426249d71353e8895b350daae2380cb26d11e0d4a01c5472"}, + {file = "asyncpg-0.28.0-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:63861bb4a540fa033a56db3bb58b0c128c56fad5d24e6d0a8c37cb29b17c1c7d"}, + {file = "asyncpg-0.28.0-cp37-cp37m-musllinux_1_1_aarch64.whl", hash = "sha256:a93a94ae777c70772073d0512f21c74ac82a8a49be3a1d982e3f259ab5f27307"}, + {file = "asyncpg-0.28.0-cp37-cp37m-musllinux_1_1_x86_64.whl", hash = "sha256:d14681110e51a9bc9c065c4e7944e8139076a778e56d6f6a306a26e740ed86d2"}, + {file = "asyncpg-0.28.0-cp37-cp37m-win32.whl", hash = "sha256:8aec08e7310f9ab322925ae5c768532e1d78cfb6440f63c078b8392a38aa636a"}, + {file = "asyncpg-0.28.0-cp37-cp37m-win_amd64.whl", hash = "sha256:319f5fa1ab0432bc91fb39b3960b0d591e6b5c7844dafc92c79e3f1bff96abef"}, + {file = "asyncpg-0.28.0-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:b337ededaabc91c26bf577bfcd19b5508d879c0ad009722be5bb0a9dd30b85a0"}, + {file = "asyncpg-0.28.0-cp38-cp38-macosx_11_0_arm64.whl", hash = "sha256:4d32b680a9b16d2957a0a3cc6b7fa39068baba8e6b728f2e0a148a67644578f4"}, + {file = "asyncpg-0.28.0-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:f4f62f04cdf38441a70f279505ef3b4eadf64479b17e707c950515846a2df197"}, + {file = "asyncpg-0.28.0-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:4f20cac332c2576c79c2e8e6464791c1f1628416d1115935a34ddd7121bfc6a4"}, + {file = "asyncpg-0.28.0-cp38-cp38-musllinux_1_1_aarch64.whl", hash = "sha256:59f9712ce01e146ff71d95d561fb68bd2d588a35a187116ef05028675462d5ed"}, + {file = "asyncpg-0.28.0-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:fc9e9f9ff1aa0eddcc3247a180ac9e9b51a62311e988809ac6152e8fb8097756"}, + {file = "asyncpg-0.28.0-cp38-cp38-win32.whl", hash = "sha256:9e721dccd3838fcff66da98709ed884df1e30a95f6ba19f595a3706b4bc757e3"}, + {file = "asyncpg-0.28.0-cp38-cp38-win_amd64.whl", hash = "sha256:8ba7d06a0bea539e0487234511d4adf81dc8762249858ed2a580534e1720db00"}, + {file = "asyncpg-0.28.0-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:d009b08602b8b18edef3a731f2ce6d3f57d8dac2a0a4140367e194eabd3de457"}, + {file = "asyncpg-0.28.0-cp39-cp39-macosx_11_0_arm64.whl", hash = "sha256:ec46a58d81446d580fb21b376ec6baecab7288ce5a578943e2fc7ab73bf7eb39"}, + {file = "asyncpg-0.28.0-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:7b48ceed606cce9e64fd5480a9b0b9a95cea2b798bb95129687abd8599c8b019"}, + {file = "asyncpg-0.28.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:8858f713810f4fe67876728680f42e93b7e7d5c7b61cf2118ef9153ec16b9423"}, + {file = "asyncpg-0.28.0-cp39-cp39-musllinux_1_1_aarch64.whl", hash = "sha256:5e18438a0730d1c0c1715016eacda6e9a505fc5aa931b37c97d928d44941b4bf"}, + {file = "asyncpg-0.28.0-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:e9c433f6fcdd61c21a715ee9128a3ca48be8ac16fa07be69262f016bb0f4dbd2"}, + {file = "asyncpg-0.28.0-cp39-cp39-win32.whl", hash = "sha256:41e97248d9076bc8e4849da9e33e051be7ba37cd507cbd51dfe4b2d99c70e3dc"}, + {file = "asyncpg-0.28.0-cp39-cp39-win_amd64.whl", hash = "sha256:3ed77f00c6aacfe9d79e9eff9e21729ce92a4b38e80ea99a58ed382f42ebd55b"}, + {file = "asyncpg-0.28.0.tar.gz", hash = "sha256:7252cdc3acb2f52feaa3664280d3bcd78a46bd6c10bfd681acfffefa1120e278"}, +] + +[package.extras] +docs = ["Sphinx (>=5.3.0,<5.4.0)", "sphinx-rtd-theme (>=1.2.2)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)"] +test = ["flake8 (>=5.0,<6.0)", "uvloop (>=0.15.3)"] + [[package]] name = "black" version = "23.9.1" @@ -1166,7 +1219,7 @@ files = [ ] [package.dependencies] -greenlet = {version = "!=0.4.17", markers = "platform_machine == \"aarch64\" or platform_machine == \"ppc64le\" or platform_machine == \"x86_64\" or platform_machine == \"amd64\" or platform_machine == \"AMD64\" or platform_machine == \"win32\" or platform_machine == \"WIN32\""} +greenlet = {version = "!=0.4.17", optional = true, markers = "platform_machine == \"aarch64\" or platform_machine == \"ppc64le\" or platform_machine == \"x86_64\" or platform_machine == \"amd64\" or platform_machine == \"AMD64\" or platform_machine == \"win32\" or platform_machine == \"WIN32\" or extra == \"asyncio\""} typing-extensions = ">=4.2.0" [package.extras] @@ -1344,4 +1397,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "c040e1afa5c08286481afc2babd402e161148b96bcce72b1d562718045e9358c" +content-hash = "ced476e10b04eb5c6461ef3caee90d57b694da635c2561eddbf7d7d6b1c44308" diff --git a/pyproject.toml b/pyproject.toml index 757c245..4e3f194 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,12 +40,13 @@ version_scheme = "no-guess-dev" [tool.poetry.dependencies] python = "^3.8" -sqlalchemy = ">=1.4" +sqlalchemy = {extras = ["asyncio"], version = ">=1.4"} strawberry-graphql = ">=0.95" sentinel = ">=0.3,<1.1" greenlet = {version = ">=3.0.0rc1", python = ">=3.12"} [tool.poetry.group.dev.dependencies] +asyncpg = "^0.28.0" black = ">=22,<24" importlib-metadata = ">=4.11.1,<7.0.0" mypy = "1.5.1" @@ -106,6 +107,8 @@ ignore = [ # we'd want to have consistent docstrings in future "D", "ANN001", # missing annotation for function argument self. + "ANN002", # missing annotation for *args. + "ANN003", # missing annotatino for **kwargs. "ANN101", # missing annotation for self? # definitely enable these, maybe not in tests "ANN102", diff --git a/src/strawberry_sqlalchemy_mapper/loader.py b/src/strawberry_sqlalchemy_mapper/loader.py index 8f0f135..c0fa908 100644 --- a/src/strawberry_sqlalchemy_mapper/loader.py +++ b/src/strawberry_sqlalchemy_mapper/loader.py @@ -1,8 +1,10 @@ +import logging from collections import defaultdict -from typing import Any, Dict, List, Mapping, Tuple, Union +from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple, Union from sqlalchemy import select, tuple_ from sqlalchemy.engine.base import Connection +from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession from sqlalchemy.orm import RelationshipProperty, Session from strawberry.dataloader import DataLoader @@ -14,9 +16,37 @@ class StrawberrySQLAlchemyLoader: _loaders: Dict[RelationshipProperty, DataLoader] - def __init__(self, bind: Union[Session, Connection]) -> None: + def __init__( + self, + bind: Union[Session, Connection, None] = None, + async_bind_factory: Optional[ + Callable[[], Union[AsyncSession, AsyncConnection]] + ] = None, + ) -> None: self._loaders = {} - self.bind = bind + self._bind = bind + self._async_bind_factory = async_bind_factory + self._logger = logging.getLogger("strawberry_sqlalchemy_mapper") + if bind is None and async_bind_factory is None: + self._logger.warning( + "One of bind or async_bind_factory must be set for loader to function properly." + ) + if bind is not None: + # For anyone coming here because of this warning: + # Making blocking database calls from within an async function (the resolver) has + # catastrophic performance implications. Not only will all resolvers be effectively + # serialized, any other coroutines waiting on the event loop (e.g. concurrent requests + # in a web server), will be blocked as well, grinding your entire service to a halt. + self._logger.warning( + "`bind` parameter is deprecated due to performance issues. Use `async_bind_factory` instead." + ) + + async def _scalars(self, *args, **kwargs): + if self._async_bind_factory: + return await self._async_bind_factory().scalars(*args, **kwargs) + else: + # Deprecated, but supported for now. + return self._bind.scalars(*args, **kwargs) def loader_for(self, relationship: RelationshipProperty) -> DataLoader: """ @@ -35,7 +65,7 @@ async def load_fn(keys: List[Tuple]) -> List[Any]: ) if relationship.order_by: query = query.order_by(*relationship.order_by) - rows = self.bind.scalars(query).all() + rows = (await self._scalars(query)).all() def group_by_remote_key(row: Any) -> Tuple: return tuple( diff --git a/tests/conftest.py b/tests/conftest.py index 51a52db..fdd665b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,6 +17,9 @@ from packaging import version from sqlalchemy import orm from sqlalchemy.engine import Engine +from sqlalchemy.ext import asyncio +from sqlalchemy.ext.asyncio import AsyncAttrs, create_async_engine +from sqlalchemy.ext.asyncio.engine import AsyncEngine from testing.postgresql import Postgresql, PostgresqlFactory SQLA_VERSION = version.parse(sqlalchemy.__version__) @@ -57,7 +60,11 @@ def postgresql(postgresql_factory) -> Postgresql: @pytest.fixture(params=SUPPORTED_DBS) def engine(request) -> Engine: if request.param == "postgresql": - url = request.getfixturevalue("postgresql").url() + url = ( + request.getfixturevalue("postgresql") + .url() + .replace("postgresql://", "postgresql+psycopg2://") + ) else: raise ValueError("Unsupported database: %s", request.param) kwargs = {} @@ -72,6 +79,31 @@ def sessionmaker(engine) -> orm.sessionmaker: return orm.sessionmaker(autocommit=False, autoflush=False, bind=engine) +@pytest.fixture(params=SUPPORTED_DBS) +def async_engine(request) -> AsyncEngine: + if request.param == "postgresql": + url = ( + request.getfixturevalue("postgresql") + .url() + .replace("postgresql://", "postgresql+asyncpg://") + ) + else: + raise ValueError("Unsupported database: %s", request.param) + kwargs = {} + if not SQLA2: + kwargs["future"] = True + engine = create_async_engine(url, **kwargs) + return engine + + +@pytest.fixture +def async_sessionmaker(async_engine) -> asyncio.async_sessionmaker: + return asyncio.async_sessionmaker(async_engine) + + @pytest.fixture def base(): - return orm.declarative_base() + class Base(AsyncAttrs, orm.DeclarativeBase): + pass + + return Base diff --git a/tests/test_loader.py b/tests/test_loader.py index 085ed0d..ee0aaa6 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -58,7 +58,7 @@ class Department(base): def test_loader_init(): loader = StrawberrySQLAlchemyLoader(bind=None) - assert loader.bind is None + assert loader._bind is None assert loader._loaders == {} @@ -99,14 +99,49 @@ async def test_loader_for(engine, base, sessionmaker, many_to_one_tables): assert department.name == "d2" loader = base_loader.loader_for(Department.employees.property) - key = tuple( + + employees = await loader.load((d2.id,)) + assert {e.name for e in employees} == {"e1"} + + +@pytest.mark.asyncio +async def test_loader_with_async_session( + async_engine, base, async_sessionmaker, many_to_one_tables +): + Employee, Department = many_to_one_tables + async with async_engine.begin() as conn: + await conn.run_sync(base.metadata.create_all) + + async with async_sessionmaker() as session: + e1 = Employee(name="e1") + e2 = Employee(name="e2") + d1 = Department(name="d1") + d2 = Department(name="d2") + session.add(e1) + session.add(e2) + session.add(d1) + session.add(d2) + await session.flush() + + e1.department = d2 + e2.department = d1 + await session.commit() + d2_id = await d2.awaitable_attrs.id + department_loader_key = tuple( [ - getattr(d2, local.key) - for local, _ in Department.employees.property.local_remote_pairs + await getattr(e1.awaitable_attrs, local.key) + for local, _ in Employee.department.property.local_remote_pairs ] ) - employees = await loader.load((d2.id,)) - assert {e.name for e in employees} == {"e1"} + base_loader = StrawberrySQLAlchemyLoader(async_bind_factory=async_sessionmaker) + loader = base_loader.loader_for(Employee.department.property) + + department = await loader.load(department_loader_key) + assert department.name == "d2" + + loader = base_loader.loader_for(Department.employees.property) + employees = await loader.load((d2_id,)) + assert {e.name for e in employees} == {"e1"} @pytest.mark.xfail From e2ace3477391fe1485f68339db38bac699399e91 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 22:35:07 +0000 Subject: [PATCH 02/16] Add assertion to make mypy happy. --- src/strawberry_sqlalchemy_mapper/loader.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/strawberry_sqlalchemy_mapper/loader.py b/src/strawberry_sqlalchemy_mapper/loader.py index c0fa908..63a4077 100644 --- a/src/strawberry_sqlalchemy_mapper/loader.py +++ b/src/strawberry_sqlalchemy_mapper/loader.py @@ -46,6 +46,7 @@ async def _scalars(self, *args, **kwargs): return await self._async_bind_factory().scalars(*args, **kwargs) else: # Deprecated, but supported for now. + assert self._bind is not None return self._bind.scalars(*args, **kwargs) def loader_for(self, relationship: RelationshipProperty) -> DataLoader: From 681dbe7a5fd831222da1ba2c9ef7b2d947aceabe Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 20 Sep 2023 23:07:50 +0000 Subject: [PATCH 03/16] Add a benchmark to test fetching of many relationships. This functions as a baseline, to confirm a bug that I believe I spotted which effectively serializes resolution of each relationship. --- tests/benchmarks/test_empty_benchmark.py | 8 -- tests/benchmarks/test_relationship_loading.py | 108 ++++++++++++++++++ 2 files changed, 108 insertions(+), 8 deletions(-) delete mode 100644 tests/benchmarks/test_empty_benchmark.py create mode 100644 tests/benchmarks/test_relationship_loading.py diff --git a/tests/benchmarks/test_empty_benchmark.py b/tests/benchmarks/test_empty_benchmark.py deleted file mode 100644 index a29b2c3..0000000 --- a/tests/benchmarks/test_empty_benchmark.py +++ /dev/null @@ -1,8 +0,0 @@ -import pytest -from pytest_codspeed.plugin import BenchmarkFixture - - -@pytest.mark.benchmark -def test_hello_world(benchmark: BenchmarkFixture): - # This is just a placeholder until we have a real benchmark. - benchmark(lambda: None) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py new file mode 100644 index 0000000..78fced6 --- /dev/null +++ b/tests/benchmarks/test_relationship_loading.py @@ -0,0 +1,108 @@ +from typing import List + +import pytest +import sqlalchemy as sa +import sqlalchemy.orm +import strawberry +import strawberry_sqlalchemy_mapper +from asgiref.sync import async_to_sync +from pytest_codspeed.plugin import BenchmarkFixture +from strawberry.types import Info + + +@pytest.mark.benchmark +def test_load_many_relationships( + benchmark: BenchmarkFixture, engine, base, sessionmaker +): + class A(base): + __tablename__ = "a" + id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) + + class B(base): + __tablename__ = "b" + id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) + + class C(base): + __tablename__ = "c" + id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) + + class D(base): + __tablename__ = "d" + id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) + + class E(base): + __tablename__ = "e" + id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) + + class Parent(base): + __tablename__ = "parent" + id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) + a_id = sa.Column(sa.Integer, sa.ForeignKey("a.id")) + b_id = sa.Column(sa.Integer, sa.ForeignKey("b.id")) + c_id = sa.Column(sa.Integer, sa.ForeignKey("c.id")) + d_id = sa.Column(sa.Integer, sa.ForeignKey("d.id")) + e_id = sa.Column(sa.Integer, sa.ForeignKey("e.id")) + a = sa.orm.relationship("A", backref="parents") + b = sa.orm.relationship("B", backref="parents") + c = sa.orm.relationship("C", backref="parents") + d = sa.orm.relationship("D", backref="parents") + e = sa.orm.relationship("E", backref="parents") + + mapper = strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyMapper() + + @mapper.type(Parent) + class StrawberryParent: + pass + + @strawberry.type + class Query: + @strawberry.field + @staticmethod + async def parents(info: Info) -> List[StrawberryParent]: + return info.context["session"].execute(sa.select(Parent)).all() + + mapper.finalize() + base.metadata.create_all(engine) + + schema = strawberry.Schema(Query) + + with sessionmaker() as session: + for _ in range(1000): + session.add(A()) + session.add(B()) + session.add(C()) + session.add(D()) + session.add(E()) + session.commit() + for i in range(10): + parent = Parent( + a_id=i * 10 + 1, + b_id=i * 10 + 1, + c_id=i * 10 + 2, + d_id=i * 10 + 3, + e_id=i * 10 + 4, + ) + session.add(parent) + session.commit() + + async def execute(): + with sessionmaker() as session: + # Notice how we use a sync session but call Strawberry's async execute. + # This is not an ideal combination, but it's certainly a common one that + # we need to support efficiently. + await schema.execute( + """ + query { + parents { + a { edges { node { id } } }, + b { edges { node { id } } }, + c { edges { node { id } } }, + d { edges { node { id } } }, + e { edges { node { id } } }, + } + } + """, + context_value={"session": session}, + ) + + benchmark(async_to_sync(execute)) From 1301bc50b8963dc6d061c0a263f53e6da6d09416 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 20 Sep 2023 23:25:55 +0000 Subject: [PATCH 04/16] Fix loader handling. --- tests/benchmarks/test_relationship_loading.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index 78fced6..ea39486 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -102,7 +102,12 @@ async def execute(): } } """, - context_value={"session": session}, + context_value={ + "session": session, + "sqlalchemy_loader": strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyLoader( + bind=session + ), + }, ) benchmark(async_to_sync(execute)) From 48afea13f2eb5d85ae45acf980b13820262e1d66 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 00:31:14 +0000 Subject: [PATCH 05/16] Fix benchmark test. --- tests/benchmarks/test_relationship_loading.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index ea39486..b052e30 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -59,7 +59,7 @@ class Query: @strawberry.field @staticmethod async def parents(info: Info) -> List[StrawberryParent]: - return info.context["session"].execute(sa.select(Parent)).all() + return info.context["session"].query(Parent).all() mapper.finalize() base.metadata.create_all(engine) @@ -90,15 +90,15 @@ async def execute(): # Notice how we use a sync session but call Strawberry's async execute. # This is not an ideal combination, but it's certainly a common one that # we need to support efficiently. - await schema.execute( + result = await schema.execute( """ query { parents { - a { edges { node { id } } }, - b { edges { node { id } } }, - c { edges { node { id } } }, - d { edges { node { id } } }, - e { edges { node { id } } }, + a { id }, + b { id }, + c { id }, + d { id }, + e { id }, } } """, @@ -109,5 +109,7 @@ async def execute(): ), }, ) + assert not result.errors + assert len(result.data["parents"]) == 10 benchmark(async_to_sync(execute)) From 4ad5bcdc05531cfec656d5820917ef9ba32ddfa7 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 00:36:43 +0000 Subject: [PATCH 06/16] 2.0 style query. --- poetry.lock | 19 ++++++++++++++++++- pyproject.toml | 1 + tests/benchmarks/test_relationship_loading.py | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/poetry.lock b/poetry.lock index 6b76d67..d6bbe71 100644 --- a/poetry.lock +++ b/poetry.lock @@ -14,6 +14,23 @@ files = [ [package.extras] test = ["coverage", "mypy", "pexpect", "ruff", "wheel"] +[[package]] +name = "asgiref" +version = "3.7.2" +description = "ASGI specs, helper code, and adapters" +optional = false +python-versions = ">=3.7" +files = [ + {file = "asgiref-3.7.2-py3-none-any.whl", hash = "sha256:89b2ef2247e3b562a16eef663bc0e2e703ec6468e2fa8a5cd61cd449786d4f6e"}, + {file = "asgiref-3.7.2.tar.gz", hash = "sha256:9e0ce3aa93a819ba5b45120216b23878cf6e8525eb3848653452b4192b92afed"}, +] + +[package.dependencies] +typing-extensions = {version = ">=4", markers = "python_version < \"3.11\""} + +[package.extras] +tests = ["mypy (>=0.800)", "pytest", "pytest-asyncio"] + [[package]] name = "asn1crypto" version = "1.5.1" @@ -1397,4 +1414,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "ced476e10b04eb5c6461ef3caee90d57b694da635c2561eddbf7d7d6b1c44308" +content-hash = "7e2aae6ab096a311b4edcf6edac91d32cac2c0c16638ca0cebb0bb559bb41a08" diff --git a/pyproject.toml b/pyproject.toml index 4e3f194..7cff4e2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,6 +64,7 @@ pytest-mypy-plugins = ">=1.10,<4.0" pytest-xdist = {extras = ["psutil"], version = "^3.1.0"} setuptools = ">=67.8.0" "testing.postgresql" = ">=1.3.0" +asgiref = "^3.7.2" [tool.black] line-length = 88 diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index b052e30..d8d97c0 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -59,7 +59,7 @@ class Query: @strawberry.field @staticmethod async def parents(info: Info) -> List[StrawberryParent]: - return info.context["session"].query(Parent).all() + return info.context["session"].scalars(sa.select(Parent)).all() mapper.finalize() base.metadata.create_all(engine) From eb84326d9bb1b6d6b8a5ad2325870e09ea1c3fd6 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 23:34:02 +0000 Subject: [PATCH 07/16] Add async benchmark. --- pyproject.toml | 5 +- src/strawberry_sqlalchemy_mapper/loader.py | 3 +- tests/benchmarks/test_relationship_loading.py | 126 +++++++++++++++--- 3 files changed, 110 insertions(+), 24 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7cff4e2..a1771e0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,6 +46,8 @@ sentinel = ">=0.3,<1.1" greenlet = {version = ">=3.0.0rc1", python = ">=3.12"} [tool.poetry.group.dev.dependencies] +"testing.postgresql" = ">=1.3.0" +asgiref = "^3.7.2" asyncpg = "^0.28.0" black = ">=22,<24" importlib-metadata = ">=4.11.1,<7.0.0" @@ -60,11 +62,10 @@ pytest-asyncio = ">=0.20.3,<0.22.0" pytest-codspeed = "^2.0.1" pytest-cov = "^4.0.0" pytest-emoji = "^0.2.0" +pytest-mock = "^3.11.1" pytest-mypy-plugins = ">=1.10,<4.0" pytest-xdist = {extras = ["psutil"], version = "^3.1.0"} setuptools = ">=67.8.0" -"testing.postgresql" = ">=1.3.0" -asgiref = "^3.7.2" [tool.black] line-length = 88 diff --git a/src/strawberry_sqlalchemy_mapper/loader.py b/src/strawberry_sqlalchemy_mapper/loader.py index 63a4077..1619a79 100644 --- a/src/strawberry_sqlalchemy_mapper/loader.py +++ b/src/strawberry_sqlalchemy_mapper/loader.py @@ -43,7 +43,8 @@ def __init__( async def _scalars(self, *args, **kwargs): if self._async_bind_factory: - return await self._async_bind_factory().scalars(*args, **kwargs) + async with self._async_bind_factory() as bind: + return await bind.scalars(*args, **kwargs) else: # Deprecated, but supported for now. assert self._bind is not None diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index d8d97c0..be8ac76 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -1,3 +1,4 @@ +import time from typing import List import pytest @@ -7,13 +8,12 @@ import strawberry_sqlalchemy_mapper from asgiref.sync import async_to_sync from pytest_codspeed.plugin import BenchmarkFixture +from sqlalchemy import orm from strawberry.types import Info -@pytest.mark.benchmark -def test_load_many_relationships( - benchmark: BenchmarkFixture, engine, base, sessionmaker -): +@pytest.fixture +def populated_tables(engine, base, sessionmaker): class A(base): __tablename__ = "a" id = sa.Column(sa.Integer, autoincrement=True, primary_key=True) @@ -48,24 +48,7 @@ class Parent(base): d = sa.orm.relationship("D", backref="parents") e = sa.orm.relationship("E", backref="parents") - mapper = strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyMapper() - - @mapper.type(Parent) - class StrawberryParent: - pass - - @strawberry.type - class Query: - @strawberry.field - @staticmethod - async def parents(info: Info) -> List[StrawberryParent]: - return info.context["session"].scalars(sa.select(Parent)).all() - - mapper.finalize() base.metadata.create_all(engine) - - schema = strawberry.Schema(Query) - with sessionmaker() as session: for _ in range(1000): session.add(A()) @@ -85,6 +68,43 @@ async def parents(info: Info) -> List[StrawberryParent]: session.add(parent) session.commit() + return A, B, C, D, E, Parent + + +@pytest.mark.benchmark +def test_load_many_relationships( + benchmark: BenchmarkFixture, populated_tables, sessionmaker, mocker +): + A, B, C, D, E, Parent = populated_tables + + mapper = strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyMapper() + + @mapper.type(Parent) + class StrawberryParent: + pass + + @strawberry.type + class Query: + @strawberry.field + @staticmethod + async def parents(info: Info) -> List[StrawberryParent]: + return info.context["session"].scalars(sa.select(Parent)).all() + + mapper.finalize() + + schema = strawberry.Schema(Query) + + # Now that we've seeded the database, let's add some delay to simulate network lag + # to the database. + old_execute_internal = orm.Session._execute_internal + mocker.patch.object(orm.Session, "_execute_internal", autospec=True) + + def sleep_then_execute(self, *args, **kwargs): + time.sleep(0.01) + return old_execute_internal(self, *args, **kwargs) + + orm.Session._execute_internal.side_effect = sleep_then_execute + async def execute(): with sessionmaker() as session: # Notice how we use a sync session but call Strawberry's async execute. @@ -113,3 +133,67 @@ async def execute(): assert len(result.data["parents"]) == 10 benchmark(async_to_sync(execute)) + + +@pytest.mark.benchmark +def test_load_many_relationships_async( + benchmark: BenchmarkFixture, populated_tables, async_sessionmaker, mocker +): + A, B, C, D, E, Parent = populated_tables + + mapper = strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyMapper() + + @mapper.type(Parent) + class StrawberryParent: + pass + + @strawberry.type + class Query: + @strawberry.field + @staticmethod + async def parents(info: Info) -> List[StrawberryParent]: + async with info.context["async_sessionmaker"]() as session: + return (await session.scalars(sa.select(Parent))).all() + + mapper.finalize() + + schema = strawberry.Schema(Query) + + # Now that we've seeded the database, let's add some delay to simulate network lag + # to the database. + old_execute_internal = orm.Session._execute_internal + mocker.patch.object(orm.Session, "_execute_internal", autospec=True) + + def sleep_then_execute(self, *args, **kwargs): + time.sleep(0.01) + return old_execute_internal(self, *args, **kwargs) + + orm.Session._execute_internal.side_effect = sleep_then_execute + + async def execute(): + # Notice how we use a sync session but call Strawberry's async execute. + # This is not an ideal combination, but it's certainly a common one that + # we need to support efficiently. + result = await schema.execute( + """ + query { + parents { + a { id }, + b { id }, + c { id }, + d { id }, + e { id }, + } + } + """, + context_value={ + "async_sessionmaker": async_sessionmaker, + "sqlalchemy_loader": strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyLoader( + async_bind_factory=async_sessionmaker + ), + }, + ) + assert not result.errors + assert len(result.data["parents"]) == 10 + + benchmark(async_to_sync(execute)) From dce028b7540a3041c9e0eb9902984ad0a8fad5c0 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 23:42:49 +0000 Subject: [PATCH 08/16] Update lock file. --- poetry.lock | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/poetry.lock b/poetry.lock index d6bbe71..58e4100 100644 --- a/poetry.lock +++ b/poetry.lock @@ -918,6 +918,23 @@ files = [ [package.dependencies] pytest = ">=4.2.1" +[[package]] +name = "pytest-mock" +version = "3.11.1" +description = "Thin-wrapper around the mock package for easier use with pytest" +optional = false +python-versions = ">=3.7" +files = [ + {file = "pytest-mock-3.11.1.tar.gz", hash = "sha256:7f6b125602ac6d743e523ae0bfa71e1a697a2f5534064528c6ff84c2f7c2fc7f"}, + {file = "pytest_mock-3.11.1-py3-none-any.whl", hash = "sha256:21c279fff83d70763b05f8874cc9cfb3fcacd6d354247a976f9529d19f9acf39"}, +] + +[package.dependencies] +pytest = ">=5.0" + +[package.extras] +dev = ["pre-commit", "pytest-asyncio", "tox"] + [[package]] name = "pytest-mypy-plugins" version = "3.0.0" @@ -1414,4 +1431,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "7e2aae6ab096a311b4edcf6edac91d32cac2c0c16638ca0cebb0bb559bb41a08" +content-hash = "dc05ea266856a08d60e5132743d5e991e38f2c184a1c48ddce948dd6a4d62aee" From 710ad24eb6fe65d404f3f8ac8e0af41bc8694f65 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 21 Sep 2023 23:57:37 +0000 Subject: [PATCH 09/16] Add proper awaitable delay. --- tests/benchmarks/test_relationship_loading.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index be8ac76..68bbd98 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -1,5 +1,7 @@ +import asyncio import time from typing import List +from unittest.mock import AsyncMock import pytest import sqlalchemy as sa @@ -9,6 +11,7 @@ from asgiref.sync import async_to_sync from pytest_codspeed.plugin import BenchmarkFixture from sqlalchemy import orm +from sqlalchemy.ext.asyncio import AsyncSession from strawberry.types import Info @@ -160,15 +163,17 @@ async def parents(info: Info) -> List[StrawberryParent]: schema = strawberry.Schema(Query) # Now that we've seeded the database, let's add some delay to simulate network lag - # to the database. - old_execute_internal = orm.Session._execute_internal - mocker.patch.object(orm.Session, "_execute_internal", autospec=True) + # to the database. We can add this lag into the + old_scalars = AsyncSession.scalars + mocker.patch.object(AsyncSession, "scalars", autospec=True) - def sleep_then_execute(self, *args, **kwargs): - time.sleep(0.01) - return old_execute_internal(self, *args, **kwargs) + async def sleep_then_scalars(self, *args, **kwargs): + await asyncio.sleep(0.01) + return await old_scalars(self, *args, **kwargs) - orm.Session._execute_internal.side_effect = sleep_then_execute + mock = AsyncMock() + mock.side_effect = sleep_then_scalars + AsyncSession.scalars.side_effect = mock async def execute(): # Notice how we use a sync session but call Strawberry's async execute. From 26856270f82bcb9ef0f9fd8dfefcf5c799840856 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 00:48:42 +0000 Subject: [PATCH 10/16] Comment out monkey patching. --- tests/benchmarks/test_relationship_loading.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index 68bbd98..bc7ca2e 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -1,7 +1,5 @@ -import asyncio import time from typing import List -from unittest.mock import AsyncMock import pytest import sqlalchemy as sa @@ -11,7 +9,6 @@ from asgiref.sync import async_to_sync from pytest_codspeed.plugin import BenchmarkFixture from sqlalchemy import orm -from sqlalchemy.ext.asyncio import AsyncSession from strawberry.types import Info @@ -162,6 +159,7 @@ async def parents(info: Info) -> List[StrawberryParent]: schema = strawberry.Schema(Query) + """ # Now that we've seeded the database, let's add some delay to simulate network lag # to the database. We can add this lag into the old_scalars = AsyncSession.scalars @@ -175,6 +173,8 @@ async def sleep_then_scalars(self, *args, **kwargs): mock.side_effect = sleep_then_scalars AsyncSession.scalars.side_effect = mock + """ + async def execute(): # Notice how we use a sync session but call Strawberry's async execute. # This is not an ideal combination, but it's certainly a common one that From 951578e11b33d60e73c759155e20b4f09df75075 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 01:18:17 +0000 Subject: [PATCH 11/16] Get rid of async_to_sync because it spawns a new event loop. --- tests/benchmarks/test_relationship_loading.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index bc7ca2e..10b35da 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -1,14 +1,16 @@ +import asyncio import time from typing import List +from unittest.mock import AsyncMock import pytest import sqlalchemy as sa import sqlalchemy.orm import strawberry import strawberry_sqlalchemy_mapper -from asgiref.sync import async_to_sync from pytest_codspeed.plugin import BenchmarkFixture from sqlalchemy import orm +from sqlalchemy.ext.asyncio import AsyncSession from strawberry.types import Info @@ -132,7 +134,7 @@ async def execute(): assert not result.errors assert len(result.data["parents"]) == 10 - benchmark(async_to_sync(execute)) + benchmark(asyncio.run, execute()) @pytest.mark.benchmark @@ -159,7 +161,6 @@ async def parents(info: Info) -> List[StrawberryParent]: schema = strawberry.Schema(Query) - """ # Now that we've seeded the database, let's add some delay to simulate network lag # to the database. We can add this lag into the old_scalars = AsyncSession.scalars @@ -173,8 +174,6 @@ async def sleep_then_scalars(self, *args, **kwargs): mock.side_effect = sleep_then_scalars AsyncSession.scalars.side_effect = mock - """ - async def execute(): # Notice how we use a sync session but call Strawberry's async execute. # This is not an ideal combination, but it's certainly a common one that @@ -201,4 +200,4 @@ async def execute(): assert not result.errors assert len(result.data["parents"]) == 10 - benchmark(async_to_sync(execute)) + benchmark(asyncio.run, execute()) From c2e8e0900a86574711102c9d86409cc40973637f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 01:27:41 +0000 Subject: [PATCH 12/16] New benchmark spawn technique. --- tests/benchmarks/test_relationship_loading.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index 10b35da..2316eb5 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -134,7 +134,10 @@ async def execute(): assert not result.errors assert len(result.data["parents"]) == 10 - benchmark(asyncio.run, execute()) + def execute_sync(): + asyncio.run(execute()) + + benchmark(execute_sync) @pytest.mark.benchmark @@ -200,4 +203,7 @@ async def execute(): assert not result.errors assert len(result.data["parents"]) == 10 - benchmark(asyncio.run, execute()) + def execute_sync(): + asyncio.run(execute()) + + benchmark(execute_sync) From de1e7992106519a895d0893324e6ed843103c520 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 01:40:11 +0000 Subject: [PATCH 13/16] Create a new async engine for each benchmark run. --- tests/benchmarks/test_relationship_loading.py | 9 ++++++--- tests/conftest.py | 13 ++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index 2316eb5..2cb9a17 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -5,6 +5,7 @@ import pytest import sqlalchemy as sa +import sqlalchemy.ext.asyncio import sqlalchemy.orm import strawberry import strawberry_sqlalchemy_mapper @@ -142,7 +143,7 @@ def execute_sync(): @pytest.mark.benchmark def test_load_many_relationships_async( - benchmark: BenchmarkFixture, populated_tables, async_sessionmaker, mocker + benchmark: BenchmarkFixture, populated_tables, async_engine_factory, mocker ): A, B, C, D, E, Parent = populated_tables @@ -181,6 +182,8 @@ async def execute(): # Notice how we use a sync session but call Strawberry's async execute. # This is not an ideal combination, but it's certainly a common one that # we need to support efficiently. + engine = async_engine_factory() + sessionmaker = sqlalchemy.ext.asyncio.async_sessionmaker(engine) result = await schema.execute( """ query { @@ -194,9 +197,9 @@ async def execute(): } """, context_value={ - "async_sessionmaker": async_sessionmaker, + "async_sessionmaker": sessionmaker, "sqlalchemy_loader": strawberry_sqlalchemy_mapper.StrawberrySQLAlchemyLoader( - async_bind_factory=async_sessionmaker + async_bind_factory=sessionmaker ), }, ) diff --git a/tests/conftest.py b/tests/conftest.py index fdd665b..72e85df 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,9 +8,11 @@ """ import contextlib +import functools import logging import platform import socket +from typing import Callable import pytest import sqlalchemy @@ -80,7 +82,8 @@ def sessionmaker(engine) -> orm.sessionmaker: @pytest.fixture(params=SUPPORTED_DBS) -def async_engine(request) -> AsyncEngine: +def async_engine_factory(request) -> Callable[[], AsyncEngine]: + """Needed to benchmark async code which can't share engines across threads.""" if request.param == "postgresql": url = ( request.getfixturevalue("postgresql") @@ -92,8 +95,12 @@ def async_engine(request) -> AsyncEngine: kwargs = {} if not SQLA2: kwargs["future"] = True - engine = create_async_engine(url, **kwargs) - return engine + return functools.partial(create_async_engine, url, **kwargs) + + +@pytest.fixture +def async_engine(async_engine_factory) -> AsyncEngine: + return async_engine_factory() @pytest.fixture From 19ef6abf84d712d97d358db166adbc5118f6595f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 01:51:05 +0000 Subject: [PATCH 14/16] Use new engine and NullPool for both sync and async. --- tests/benchmarks/test_relationship_loading.py | 7 +++++-- tests/conftest.py | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index 2cb9a17..5c432d7 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -12,6 +12,7 @@ from pytest_codspeed.plugin import BenchmarkFixture from sqlalchemy import orm from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.pool import NullPool from strawberry.types import Info @@ -76,7 +77,7 @@ class Parent(base): @pytest.mark.benchmark def test_load_many_relationships( - benchmark: BenchmarkFixture, populated_tables, sessionmaker, mocker + benchmark: BenchmarkFixture, populated_tables, engine_factory, mocker ): A, B, C, D, E, Parent = populated_tables @@ -109,6 +110,8 @@ def sleep_then_execute(self, *args, **kwargs): orm.Session._execute_internal.side_effect = sleep_then_execute async def execute(): + engine = engine_factory(poolclass=NullPool) + sessionmaker = orm.sessionmaker(autocommit=False, autoflush=False, bind=engine) with sessionmaker() as session: # Notice how we use a sync session but call Strawberry's async execute. # This is not an ideal combination, but it's certainly a common one that @@ -182,7 +185,7 @@ async def execute(): # Notice how we use a sync session but call Strawberry's async execute. # This is not an ideal combination, but it's certainly a common one that # we need to support efficiently. - engine = async_engine_factory() + engine = async_engine_factory(poolclass=NullPool) sessionmaker = sqlalchemy.ext.asyncio.async_sessionmaker(engine) result = await schema.execute( """ diff --git a/tests/conftest.py b/tests/conftest.py index 72e85df..60c4dd7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -60,7 +60,7 @@ def postgresql(postgresql_factory) -> Postgresql: @pytest.fixture(params=SUPPORTED_DBS) -def engine(request) -> Engine: +def engine_factory(request) -> Engine: if request.param == "postgresql": url = ( request.getfixturevalue("postgresql") @@ -72,8 +72,12 @@ def engine(request) -> Engine: kwargs = {} if not SQLA2: kwargs["future"] = True - engine = sqlalchemy.create_engine(url, **kwargs) - return engine + return functools.partial(sqlalchemy.create_engine, url, **kwargs) + + +@pytest.fixture +def engine(engine_factory) -> Engine: + return engine_factory() @pytest.fixture From ca0b6373342e4ad5dc891c79aed911f904e28024 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 02:08:43 +0000 Subject: [PATCH 15/16] Use psycopg3 to have the same db driver for both sync and aysnc bencmarks. --- poetry.lock | 75 +++++++++++++++++++++++++++++++++++++---------- pyproject.toml | 3 +- tests/conftest.py | 4 +-- 3 files changed, 63 insertions(+), 19 deletions(-) diff --git a/poetry.lock b/poetry.lock index 58e4100..cffdd9a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -110,6 +110,34 @@ files = [ docs = ["Sphinx (>=5.3.0,<5.4.0)", "sphinx-rtd-theme (>=1.2.2)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)"] test = ["flake8 (>=5.0,<6.0)", "uvloop (>=0.15.3)"] +[[package]] +name = "backports-zoneinfo" +version = "0.2.1" +description = "Backport of the standard library zoneinfo module" +optional = false +python-versions = ">=3.6" +files = [ + {file = "backports.zoneinfo-0.2.1-cp36-cp36m-macosx_10_14_x86_64.whl", hash = "sha256:da6013fd84a690242c310d77ddb8441a559e9cb3d3d59ebac9aca1a57b2e18bc"}, + {file = "backports.zoneinfo-0.2.1-cp36-cp36m-manylinux1_i686.whl", hash = "sha256:89a48c0d158a3cc3f654da4c2de1ceba85263fafb861b98b59040a5086259722"}, + {file = "backports.zoneinfo-0.2.1-cp36-cp36m-manylinux1_x86_64.whl", hash = "sha256:1c5742112073a563c81f786e77514969acb58649bcdf6cdf0b4ed31a348d4546"}, + {file = "backports.zoneinfo-0.2.1-cp36-cp36m-win32.whl", hash = "sha256:e8236383a20872c0cdf5a62b554b27538db7fa1bbec52429d8d106effbaeca08"}, + {file = "backports.zoneinfo-0.2.1-cp36-cp36m-win_amd64.whl", hash = "sha256:8439c030a11780786a2002261569bdf362264f605dfa4d65090b64b05c9f79a7"}, + {file = "backports.zoneinfo-0.2.1-cp37-cp37m-macosx_10_14_x86_64.whl", hash = "sha256:f04e857b59d9d1ccc39ce2da1021d196e47234873820cbeaad210724b1ee28ac"}, + {file = "backports.zoneinfo-0.2.1-cp37-cp37m-manylinux1_i686.whl", hash = "sha256:17746bd546106fa389c51dbea67c8b7c8f0d14b5526a579ca6ccf5ed72c526cf"}, + {file = "backports.zoneinfo-0.2.1-cp37-cp37m-manylinux1_x86_64.whl", hash = "sha256:5c144945a7752ca544b4b78c8c41544cdfaf9786f25fe5ffb10e838e19a27570"}, + {file = "backports.zoneinfo-0.2.1-cp37-cp37m-win32.whl", hash = "sha256:e55b384612d93be96506932a786bbcde5a2db7a9e6a4bb4bffe8b733f5b9036b"}, + {file = "backports.zoneinfo-0.2.1-cp37-cp37m-win_amd64.whl", hash = "sha256:a76b38c52400b762e48131494ba26be363491ac4f9a04c1b7e92483d169f6582"}, + {file = "backports.zoneinfo-0.2.1-cp38-cp38-macosx_10_14_x86_64.whl", hash = "sha256:8961c0f32cd0336fb8e8ead11a1f8cd99ec07145ec2931122faaac1c8f7fd987"}, + {file = "backports.zoneinfo-0.2.1-cp38-cp38-manylinux1_i686.whl", hash = "sha256:e81b76cace8eda1fca50e345242ba977f9be6ae3945af8d46326d776b4cf78d1"}, + {file = "backports.zoneinfo-0.2.1-cp38-cp38-manylinux1_x86_64.whl", hash = "sha256:7b0a64cda4145548fed9efc10322770f929b944ce5cee6c0dfe0c87bf4c0c8c9"}, + {file = "backports.zoneinfo-0.2.1-cp38-cp38-win32.whl", hash = "sha256:1b13e654a55cd45672cb54ed12148cd33628f672548f373963b0bff67b217328"}, + {file = "backports.zoneinfo-0.2.1-cp38-cp38-win_amd64.whl", hash = "sha256:4a0f800587060bf8880f954dbef70de6c11bbe59c673c3d818921f042f9954a6"}, + {file = "backports.zoneinfo-0.2.1.tar.gz", hash = "sha256:fadbfe37f74051d024037f223b8e001611eac868b5c5b06144ef4d8b799862f2"}, +] + +[package.extras] +tzdata = ["tzdata"] + [[package]] name = "black" version = "23.9.1" @@ -794,25 +822,29 @@ files = [ test = ["enum34", "ipaddress", "mock", "pywin32", "wmi"] [[package]] -name = "psycopg2" -version = "2.9.7" -description = "psycopg2 - Python-PostgreSQL Database Adapter" +name = "psycopg" +version = "3.1.10" +description = "PostgreSQL database adapter for Python" optional = false -python-versions = ">=3.6" +python-versions = ">=3.7" files = [ - {file = "psycopg2-2.9.7-cp310-cp310-win32.whl", hash = "sha256:1a6a2d609bce44f78af4556bea0c62a5e7f05c23e5ea9c599e07678995609084"}, - {file = "psycopg2-2.9.7-cp310-cp310-win_amd64.whl", hash = "sha256:b22ed9c66da2589a664e0f1ca2465c29b75aaab36fa209d4fb916025fb9119e5"}, - {file = "psycopg2-2.9.7-cp311-cp311-win32.whl", hash = "sha256:44d93a0109dfdf22fe399b419bcd7fa589d86895d3931b01fb321d74dadc68f1"}, - {file = "psycopg2-2.9.7-cp311-cp311-win_amd64.whl", hash = "sha256:91e81a8333a0037babfc9fe6d11e997a9d4dac0f38c43074886b0d9dead94fe9"}, - {file = "psycopg2-2.9.7-cp37-cp37m-win32.whl", hash = "sha256:d1210fcf99aae6f728812d1d2240afc1dc44b9e6cba526a06fb8134f969957c2"}, - {file = "psycopg2-2.9.7-cp37-cp37m-win_amd64.whl", hash = "sha256:e9b04cbef584310a1ac0f0d55bb623ca3244c87c51187645432e342de9ae81a8"}, - {file = "psycopg2-2.9.7-cp38-cp38-win32.whl", hash = "sha256:d5c5297e2fbc8068d4255f1e606bfc9291f06f91ec31b2a0d4c536210ac5c0a2"}, - {file = "psycopg2-2.9.7-cp38-cp38-win_amd64.whl", hash = "sha256:8275abf628c6dc7ec834ea63f6f3846bf33518907a2b9b693d41fd063767a866"}, - {file = "psycopg2-2.9.7-cp39-cp39-win32.whl", hash = "sha256:c7949770cafbd2f12cecc97dea410c514368908a103acf519f2a346134caa4d5"}, - {file = "psycopg2-2.9.7-cp39-cp39-win_amd64.whl", hash = "sha256:b6bd7d9d3a7a63faae6edf365f0ed0e9b0a1aaf1da3ca146e6b043fb3eb5d723"}, - {file = "psycopg2-2.9.7.tar.gz", hash = "sha256:f00cc35bd7119f1fed17b85bd1007855194dde2cbd8de01ab8ebb17487440ad8"}, + {file = "psycopg-3.1.10-py3-none-any.whl", hash = "sha256:8bbeddae5075c7890b2fa3e3553440376d3c5e28418335dee3c3656b06fa2b52"}, + {file = "psycopg-3.1.10.tar.gz", hash = "sha256:15b25741494344c24066dc2479b0f383dd1b82fa5e75612fa4fa5bb30726e9b6"}, ] +[package.dependencies] +"backports.zoneinfo" = {version = ">=0.2.0", markers = "python_version < \"3.9\""} +typing-extensions = ">=4.1" +tzdata = {version = "*", markers = "sys_platform == \"win32\""} + +[package.extras] +binary = ["psycopg-binary (==3.1.10)"] +c = ["psycopg-c (==3.1.10)"] +dev = ["black (>=23.1.0)", "dnspython (>=2.1)", "flake8 (>=4.0)", "mypy (>=1.4.1)", "types-setuptools (>=57.4)", "wheel (>=0.37)"] +docs = ["Sphinx (>=5.0)", "furo (==2022.6.21)", "sphinx-autobuild (>=2021.3.14)", "sphinx-autodoc-typehints (>=1.12)"] +pool = ["psycopg-pool"] +test = ["anyio (>=3.6.2)", "mypy (>=1.4.1)", "pproxy (>=2.7)", "pytest (>=6.2.5)", "pytest-cov (>=3.0)", "pytest-randomly (>=3.5)"] + [[package]] name = "pycparser" version = "2.21" @@ -1379,6 +1411,17 @@ files = [ {file = "typing_extensions-4.7.1.tar.gz", hash = "sha256:b75ddc264f0ba5615db7ba217daeb99701ad295353c45f9e95963337ceeeffb2"}, ] +[[package]] +name = "tzdata" +version = "2023.3" +description = "Provider of IANA time zone data" +optional = false +python-versions = ">=2" +files = [ + {file = "tzdata-2023.3-py2.py3-none-any.whl", hash = "sha256:7e65763eef3120314099b6939b5546db7adce1e7d6f2e179e3df563c70511eda"}, + {file = "tzdata-2023.3.tar.gz", hash = "sha256:11ef1e08e54acb0d4f95bdb1be05da659673de4acbd21bf9c69e94cc5e907a3a"}, +] + [[package]] name = "virtualenv" version = "20.24.4" @@ -1431,4 +1474,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "dc05ea266856a08d60e5132743d5e991e38f2c184a1c48ddce948dd6a4d62aee" +content-hash = "557794c8a7cbc72d338b6b8c54dc9d8e4110003ddefb857d845ad7da0482017d" diff --git a/pyproject.toml b/pyproject.toml index a1771e0..cb29f1e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,7 +56,7 @@ nox = "^2023.4.22" nox-poetry = "^1.0.2" packaging = ">=23.1" pg8000 = ">=1.30.1" -psycopg2 = ">=2.9.7" +psycopg = "^3.1" pytest = "^7.2" pytest-asyncio = ">=0.20.3,<0.22.0" pytest-codspeed = "^2.0.1" @@ -66,6 +66,7 @@ pytest-mock = "^3.11.1" pytest-mypy-plugins = ">=1.10,<4.0" pytest-xdist = {extras = ["psutil"], version = "^3.1.0"} setuptools = ">=67.8.0" +sqlalchemy = {extras = ["asyncio"], version = ">=2.0"} [tool.black] line-length = 88 diff --git a/tests/conftest.py b/tests/conftest.py index 60c4dd7..2ddd30e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -65,7 +65,7 @@ def engine_factory(request) -> Engine: url = ( request.getfixturevalue("postgresql") .url() - .replace("postgresql://", "postgresql+psycopg2://") + .replace("postgresql://", "postgresql+psycopg://") ) else: raise ValueError("Unsupported database: %s", request.param) @@ -92,7 +92,7 @@ def async_engine_factory(request) -> Callable[[], AsyncEngine]: url = ( request.getfixturevalue("postgresql") .url() - .replace("postgresql://", "postgresql+asyncpg://") + .replace("postgresql://", "postgresql+psycopg://") ) else: raise ValueError("Unsupported database: %s", request.param) From c1382fdbf8a2ed9662f242c9cdb99f60b718b6fa Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 22 Sep 2023 02:22:42 +0000 Subject: [PATCH 16/16] Increase sleep time to see if that changes benchmarks. --- tests/benchmarks/test_relationship_loading.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/benchmarks/test_relationship_loading.py b/tests/benchmarks/test_relationship_loading.py index 5c432d7..f95985c 100644 --- a/tests/benchmarks/test_relationship_loading.py +++ b/tests/benchmarks/test_relationship_loading.py @@ -104,7 +104,7 @@ async def parents(info: Info) -> List[StrawberryParent]: mocker.patch.object(orm.Session, "_execute_internal", autospec=True) def sleep_then_execute(self, *args, **kwargs): - time.sleep(0.01) + time.sleep(0.1) return old_execute_internal(self, *args, **kwargs) orm.Session._execute_internal.side_effect = sleep_then_execute @@ -174,7 +174,7 @@ async def parents(info: Info) -> List[StrawberryParent]: mocker.patch.object(AsyncSession, "scalars", autospec=True) async def sleep_then_scalars(self, *args, **kwargs): - await asyncio.sleep(0.01) + await asyncio.sleep(0.1) return await old_scalars(self, *args, **kwargs) mock = AsyncMock()