diff --git a/docs/09-with-pi-pilgrim.md b/docs/09-with-pi-pilgrim.md index 58d4c90..b79c7e1 100644 --- a/docs/09-with-pi-pilgrim.md +++ b/docs/09-with-pi-pilgrim.md @@ -49,8 +49,12 @@ app = FastAPI(lifespan=lifespan) `await AssemblyHolder.aclose()`. - **Entering your assembly doesn't start Drivine**, as entering `DrivineAssembly` itself does: call `await assembly.drivine.start()`, as above. -- **The sync assembly**, `drivine.sync.DrivineAssembly`, closes with `with` or `close()`. The async one closes only - with `async with` or `await aclose()`, as its drivers close asynchronously: `close()` refuses, and says so. +- **The async assembly is an `AsyncPilgrimAssembly`**, as its drivers close asynchronously: `await close()`, + `asyncio.run(close())`, `await aclose()` and `async with` all close it, and `with` refuses, and says to use + `async with`. Await `close()`: left unawaited, it closes nothing, and Python warns that a coroutine was never + awaited. Your assembly closes it whether it holds it with `AssemblyHolder.shared(DrivineAssembly)` or as a component, + `self.shared(DrivineAssembly)`. The sync assembly, `drivine.sync.DrivineAssembly`, is a plain `PilgrimAssembly`, + which closes with `with` or `close()`. - It registers the database the environment describes. To register yours in code, subclass it and override `register_databases(registry)`. - `Assembled` injects a manager into a class something else makes: @@ -58,3 +62,22 @@ app = FastAPI(lifespan=lifespan) The managers are plain classes, so an application with another container, or none, makes them itself, and closes the registry with `async with DatabaseRegistry() as registry:`. + +### Upgrading from 0.2.0 + +0.3.0 moved `DrivineAssembly` onto pi-pilgrim 0.3's lifecycle: the assembly closes when you leave `async with`, or when +the application assembly that holds it closes, and closing it closes every database's driver and pool. + +- **Shutdown.** 0.2.0 said to call `await assembly.drivine.close()` when the application stops. That closes everything + again in every release after 0.3.0, where `DrivineAssembly` is pi-pilgrim's `AsyncPilgrimAssembly`, whose `close()` + is `aclose()` (0.3.0 raised `PilgrimError`, or `TypeError` when Drivine had made no registry). Await it: called + without `await`, it closes nothing, and Python warns that a coroutine was never awaited. The 0.3 way is to leave your + assembly's `async with`, as in the lifespan above, or to call `await assembly.aclose()`, which closes your components + and then Drivine. To close only Drivine, `await assembly.drivine.aclose()`. +- **`with DrivineAssembly()`** raises `AsyncTeardownRequired`, a `PilgrimError`: use `async with`, or + `drivine.sync.DrivineAssembly` for the sync API, which closes with `with` or `close()`. +- **Sync `close()` on your assembly** closes Drivine while it has made no registry, whether your assembly holds it with + `AssemblyHolder.shared` or with `self.shared(DrivineAssembly)`. Once Drivine has made its registry it refuses with + `AsyncTeardownRequired`, before closing anything, since the drivers close asynchronously: close with + `await aclose()`. It refuses from the start if your subclass of `DrivineAssembly` overrides `aclose()`, which a sync + close can't run. diff --git a/docs/design.md b/docs/design.md index 0bf7ebd..87ec346 100644 --- a/docs/design.md +++ b/docs/design.md @@ -563,12 +563,13 @@ class AppAssembly(PilgrimAssembly): - `Assembled` injects Drivine's components into classes made by a framework, by type or by key: `gom: StatelessGraphObjectManager = Assembled()`. - Closing is pi-pilgrim's, as Spring closes drivine4j's beans with its context. The registry is a `shared` component, - and its `close` is the teardown pi-pilgrim infers, so closing the assembly (`await aclose()`, or leaving - `async with`) closes every driver and pool, and an application's assembly closes the `DrivineAssembly` it asked - `AssemblyHolder` for. `DrivineAssembly` has no `close` of its own: an async one would hide pi-pilgrim's sync - `close()`, and `with` would get a coroutine it never awaits. Entering `DrivineAssembly` calls `start()`, as Spring - starts drivine4j on refresh; entering an application's assembly doesn't, so the application calls - `drivine.start()`. + and its `close` is the teardown pi-pilgrim infers, so closing the assembly (`await aclose()`, or leaving `async with`) + closes every driver and pool, and an application's assembly closes the `DrivineAssembly` it asked `AssemblyHolder` + for, or holds as a component. The async `DrivineAssembly` is pi-pilgrim's `AsyncPilgrimAssembly`, whose `close()` is + `aclose()`, so 0.2.0's `await drivine.close()` closes it, and whose `with` refuses, as its exit couldn't await; it + overrides `__enter__` only to name the sync API in that refusal. The sync one is a plain `PilgrimAssembly`. Entering + `DrivineAssembly` calls `start()`, as Spring starts drivine4j on refresh; entering an application's assembly doesn't, + so the application calls `drivine.start()`. - The managers are plain classes, so an application with another container, or none, constructs them itself. `DatabaseRegistry` is a context manager for it: `async with DatabaseRegistry() as registry:` closes it. diff --git a/drivine/drivine_assembly.py b/drivine/drivine_assembly.py index 9563038..bd33420 100644 --- a/drivine/drivine_assembly.py +++ b/drivine/drivine_assembly.py @@ -1,8 +1,8 @@ from __future__ import annotations -from typing import Self +from typing import NoReturn, Self -from pipilgrim import PilgrimAssembly +from pipilgrim import AsyncPilgrimAssembly, AsyncTeardownRequired from drivine.connection.database_registry import DEFAULT, DatabaseRegistry from drivine.dsl.index_advice_policy import IndexAdvicePolicy @@ -15,7 +15,7 @@ from drivine.schema.schema_options import SchemaOptions -class DrivineAssembly(PilgrimAssembly): +class DrivineAssembly(AsyncPilgrimAssembly): """ Drivine's components: the registry and the managers. On its own: @@ -124,3 +124,10 @@ async def __aenter__(self) -> Self: await self.aclose() raise return self + + def __enter__(self) -> NoReturn: + """Refuses, as pi-pilgrim's does, and names the sync API, whose assembly `with` closes.""" + raise AsyncTeardownRequired( + f"{type(self).__name__} closes its drivers and pools asynchronously: use async with, or " + f"drivine.sync.DrivineAssembly for the sync API." + ) diff --git a/pyproject.toml b/pyproject.toml index 76ab8c0..9361f9c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ authors = [{ name = "Liberation Data" }] requires-python = ">=3.11" dependencies = [ "pydantic>=2.7", - "pi-pilgrim>=0.3.0", + "pi-pilgrim>=0.4.0,<0.5", ] keywords = ["graph", "database", "neo4j", "memgraph", "falkordb", "neptune", "oracle", "postgres", "cypher", "ogm"] classifiers = [ diff --git a/scripts/unasync.py b/scripts/unasync.py index 4a8801c..e1e6850 100644 --- a/scripts/unasync.py +++ b/scripts/unasync.py @@ -8,7 +8,8 @@ A module is written again when it has async code, or imports one that does; the rest (models, the query DSL, the backends) is shared by both APIs. drivine/concurrency.py, what the async API asks of asyncio, has a counterpart written -by hand: drivine/sync/concurrency.py. A test marked `async_only` (one that runs tasks concurrently) is left out. +by hand: drivine/sync/concurrency.py. A test marked `async_only` (one that runs tasks concurrently) is left out, as +is a definition ASYNC_ONLY names. """ from __future__ import annotations @@ -48,6 +49,13 @@ class Tree: EXTERNAL = {"falkordb.asyncio": "falkordb"} """Driver modules whose sync counterpart is another module.""" +ASYNC_ONLY = {"drivine.drivine_assembly": ("DrivineAssembly.__enter__",)} +""" +Definitions left out of the sync code, by module: the async API's own. The async DrivineAssembly's `__enter__` refuses +`with`, naming the sync API. Made sync, it would replace the `__enter__` that `__aenter__` becomes, with which the sync +assembly, a plain PilgrimAssembly, starts. +""" + RENAMES = { "AsyncIterator": "Iterator", "AsyncIterable": "Iterable", @@ -73,6 +81,7 @@ class Tree: "AsyncConnection": "Connection", "AsyncCursor": "Cursor", "AsyncIteratorFixture": "IteratorFixture", + "AsyncPilgrimAssembly": "PilgrimAssembly", } """Names whose sync counterpart is another name: the language's, and each driver's.""" @@ -137,16 +146,34 @@ def written_again(sources: dict[str, ast.Module]) -> set[str]: marked |= more -def without_async_only(text: str, source: ast.Module) -> str: - """The text without the tests marked `async_only`.""" +def without_async_only(text: str, source: ast.Module, module: str = "") -> str: + """The text without the tests marked `async_only`, or the module's definitions that ASYNC_ONLY names.""" lines = text.splitlines(keepends=True) dropped: set[int] = set() - for it in ast.walk(source): - if isinstance(it, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)) and any( - ast.unparse(decorator).endswith("mark.async_only") for decorator in it.decorator_list - ): - first = min(decorator.lineno for decorator in it.decorator_list) - dropped.update(range(first - 1, (it.end_lineno or it.lineno))) + named = ASYNC_ONLY.get(module, ()) + found: set[str] = set() + definitions = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef) + + def visit(body: list[ast.stmt], prefix: str) -> None: + for it in body: + if not isinstance(it, definitions): + continue + name = f"{prefix}{it.name}" + if name in named: + found.add(name) + if name in named or any( + ast.unparse(decorator).endswith("mark.async_only") for decorator in it.decorator_list + ): + first = min([it.lineno, *(decorator.lineno for decorator in it.decorator_list)]) + dropped.update(range(first - 1, (it.end_lineno or it.lineno))) + elif isinstance(it, ast.ClassDef): + visit(it.body, f"{name}.") + + visit(source.body, "") + missing = [it for it in named if it not in found] + if missing: + # A renamed definition would otherwise be written into the sync code without a word. + raise SystemExit(f"ASYNC_ONLY names {', '.join(missing)} in {module}, which has no such definition.") return "".join(line for i, line in enumerate(lines) if i not in dropped) @@ -252,7 +279,7 @@ def write(into: Path) -> list[Path]: written: list[Path] = [] for name in sorted(generated): path = paths[name] - text = rewritten(without_async_only(path.read_text(), sources[name]), generated) + text = rewritten(without_async_only(path.read_text(), sources[name], name), generated) left = leftovers(text) if left: raise SystemExit(f"{path.relative_to(ROOT)} still holds {', '.join(left)} once made sync.") diff --git a/tests/unit/test_assembly_lifecycle.py b/tests/unit/test_assembly_lifecycle.py index f21899f..a139524 100644 --- a/tests/unit/test_assembly_lifecycle.py +++ b/tests/unit/test_assembly_lifecycle.py @@ -3,10 +3,12 @@ closes drivine4j's with its context; so does a DatabaseRegistry that was entered. Async and sync alike. """ +import asyncio +import gc from collections.abc import Iterator import pytest -from pipilgrim import AssemblyHolder, PilgrimAssembly, PilgrimError +from pipilgrim import AssemblyHolder, AsyncPilgrimAssembly, AsyncTeardownRequired, PilgrimAssembly, PilgrimError from drivine import DatabaseRegistry, DatabaseType, DrivineAssembly from drivine.sync import ConnectionProvider as SyncConnectionProvider @@ -135,14 +137,152 @@ def start(self) -> None: assert drivine.provider.ended == 1 -async def test_the_async_assembly_refuses_a_sync_close(): +async def test_await_close_closes_the_providers_as_in_0_2_0(): + drivine = FakeDrivine() + drivine.persistence_manager() + await drivine.close() + assert drivine.provider.ended == 1 + with pytest.raises(PilgrimError, match="is closed"): + drivine.persistence_manager() + + +async def test_await_close_twice_ends_the_providers_once(): drivine = FakeDrivine() drivine.registry() - with pytest.raises(PilgrimError, match="aclose"): - drivine.close() + await drivine.close() + await drivine.close() + await drivine.aclose() + assert drivine.provider.ended == 1 + + +async def test_await_close_without_a_registry_closes_the_assembly(): + drivine = FakeDrivine() + await drivine.close() + await drivine.close() assert drivine.provider.ended == 0 + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + +async def test_await_close_after_aclose_does_nothing(): + drivine = FakeDrivine() + drivine.registry() await drivine.aclose() assert drivine.provider.ended == 1 + await drivine.close() + assert drivine.provider.ended == 1 + + +@pytest.mark.parametrize("registry", [False, True], ids=["no registry", "registry"]) +def test_asyncio_run_takes_what_close_returns(registry: bool): + drivine = FakeDrivine() + if registry: + drivine.registry() + asyncio.run(drivine.close()) + assert drivine.provider.ended == (1 if registry else 0) + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + +def test_drivine_is_an_async_pilgrim_assembly_and_the_sync_one_a_plain_one(): + assert issubclass(DrivineAssembly, AsyncPilgrimAssembly) + assert issubclass(SyncDrivineAssembly, PilgrimAssembly) + assert not issubclass(SyncDrivineAssembly, AsyncPilgrimAssembly) + + +@pytest.mark.parametrize("registry", [False, True], ids=["no registry", "registry"]) +def test_close_not_awaited_closes_nothing_and_python_warns(registry: bool): + # close() is a native coroutine function, as pi-pilgrim's AsyncPilgrimAssembly makes it: unawaited, its body never + # runs, whether or not the registry was made. + drivine = FakeDrivine() + if registry: + drivine.registry() + with pytest.warns(RuntimeWarning, match="never awaited"): + drivine.close() + gc.collect() + assert drivine.provider.ended == 0 + drivine.registry() + asyncio.run(drivine.aclose()) + assert drivine.provider.ended == 1 + + +class Holder(PilgrimAssembly): + """Holds Drivine as a component, rather than with AssemblyHolder.shared.""" + + def drivine(self) -> FakeDrivine: + return self.shared(FakeDrivine) + + +def test_a_sync_close_of_an_assembly_holding_drivine_as_a_component_closes_a_drivine_with_no_registry(): + # pi-pilgrim closes a held assembly through its own lifecycle: nothing async was made, so a sync close closes it. + holder = Holder() + drivine = holder.drivine() + holder.close() + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + +def test_a_sync_close_of_an_assembly_holding_drivine_as_a_component_refuses_once_drivine_has_a_registry(): + holder = Holder() + drivine = holder.drivine() + drivine.registry() + with pytest.raises(AsyncTeardownRequired, match="aclose"): + holder.close() + # Refused before closing anything: the holder, Drivine and its providers are still open. + assert holder.drivine() is drivine + assert drivine.provider.ended == 0 + asyncio.run(holder.aclose()) + assert drivine.provider.ended == 1 + + +async def test_await_aclose_on_an_assembly_holding_drivine_as_a_component_closes_it(): + holder = Holder() + drivine = holder.drivine() + drivine.registry() + await holder.aclose() + assert drivine.provider.ended == 1 + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + +def test_the_sync_assembly_closes_with_close(): + drivine = SyncFakeDrivine() + drivine.registry() + assert drivine.close() is None + drivine.close() + assert drivine.provider.ended == 1 + + +async def test_the_async_assembly_refuses_with_and_names_the_sync_api(): + drivine = FakeDrivine() + with pytest.raises(AsyncTeardownRequired, match=r"async with, or drivine\.sync\.DrivineAssembly"): + with drivine: # type: ignore[misc] + pytest.fail("the body ran") + assert drivine.started == 0 + await drivine.aclose() + + +async def test_await_close_ends_every_provider_when_one_fails(): + class FailingProvider(EndingProvider): + async def end(self) -> None: + await super().end() + raise RuntimeError("pool won't close") + + class TwoDatabases(FakeDrivine): + def __init__(self) -> None: + self.failing = FailingProvider("failing") + super().__init__() + + def register_databases(self, registry: DatabaseRegistry) -> None: + registry.register(self.failing) + super().register_databases(registry) + + drivine = TwoDatabases() + drivine.registry() + await drivine.close() + assert (drivine.failing.ended, drivine.provider.ended) == (1, 1) + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() class App(PilgrimAssembly): @@ -174,6 +314,75 @@ async def test_an_application_assembly_closes_the_drivine_it_holds(): app.drivine.persistence_manager() +async def test_await_close_on_an_application_assemblys_drivine_closes_it_as_in_0_2_0(): + # 0.2.0's README: `self.drivine = AssemblyHolder.shared(DrivineAssembly)`, then `await assembly.drivine.close()`. + app = App() + app.greeter() + await app.drivine.close() + assert app.drivine.provider.ended == 1 + await app.aclose() + assert app.drivine.provider.ended == 1 + + +def test_a_sync_close_of_an_application_assembly_closes_a_drivine_with_no_registry(): + # pi-pilgrim closes the DrivineAssembly it owns through its own lifecycle: nothing async was made, so it closes. + app = App() + drivine = app.drivine + app.close() + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + +def test_a_sync_close_of_an_application_assembly_refuses_once_drivine_has_a_registry(): + app = App() + app.greeter() + with pytest.raises(AsyncTeardownRequired, match="aclose"): + app.close() + assert app.drivine.provider.ended == 0 + asyncio.run(app.aclose()) + assert app.drivine.provider.ended == 1 + + +class Flushing(FakeDrivine): + """Overrides aclose(), which a sync close can't run.""" + + def __init__(self) -> None: + self.flushed = 0 + super().__init__() + + async def aclose(self) -> None: + self.flushed += 1 + await super().aclose() + + +class FlushingApp(PilgrimAssembly): + def __init__(self) -> None: + self.drivine = AssemblyHolder.shared(Flushing) + super().__init__() + + +async def test_a_drivine_that_overrides_aclose_runs_it_on_await_close_and_refuses_a_sync_close_with_no_registry(): + app = FlushingApp() + # pi-pilgrim's sync close skips an overridden aclose(), so it refuses, though nothing async was made. + with pytest.raises(AsyncTeardownRequired, match=r"Flushing\.aclose\(\)"): + app.close() + assert app.drivine.flushed == 0 + await app.drivine.close() + assert app.drivine.flushed == 1 + await app.aclose() + + +@pytest.mark.parametrize("registry", [False, True], ids=["no registry", "registry"]) +async def test_await_assembly_holder_aclose_closes_the_drivine_it_holds(registry: bool): + drivine = AssemblyHolder.shared(FakeDrivine) + if registry: + drivine.registry() + await AssemblyHolder.aclose() + assert drivine.provider.ended == (1 if registry else 0) + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + def test_a_sync_application_assembly_closes_the_drivine_it_holds(): with SyncApp() as app: app.greeter() diff --git a/tests/unit/test_unasync.py b/tests/unit/test_unasync.py new file mode 100644 index 0000000..fc44c55 --- /dev/null +++ b/tests/unit/test_unasync.py @@ -0,0 +1,52 @@ +"""scripts/unasync.py: what it leaves out of the sync code.""" + +from __future__ import annotations + +import ast +import importlib.util +import sys +from pathlib import Path + +import pytest + +_path = Path(__file__).parents[2] / "scripts" / "unasync.py" +_spec = importlib.util.spec_from_file_location("unasync", _path) +assert _spec is not None and _spec.loader is not None +unasync = importlib.util.module_from_spec(_spec) +sys.modules["unasync"] = unasync # its dataclass looks itself up there +_spec.loader.exec_module(unasync) + +SOURCE = """class Kept: + def close(self): + return 1 + + def keep(self): + return 2 + + +class Assembly: + @property + def close(self): + return 3 + + def keep(self): + return 4 + + +def close(): + return 5 +""" + + +def test_async_only_leaves_out_exactly_the_named_definitions(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(unasync, "ASYNC_ONLY", {"m": ("Assembly.close",)}) + text = unasync.without_async_only(SOURCE, ast.parse(SOURCE), "m") + assert "return 3" not in text and "@property" not in text + assert all(f"return {it}" in text for it in (1, 2, 4, 5)) + assert unasync.without_async_only(SOURCE, ast.parse(SOURCE), "other") == SOURCE + + +def test_async_only_refuses_a_name_the_module_does_not_define(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(unasync, "ASYNC_ONLY", {"m": ("Assembly.closed",)}) + with pytest.raises(SystemExit, match="Assembly.closed"): + unasync.without_async_only(SOURCE, ast.parse(SOURCE), "m") diff --git a/uv.lock b/uv.lock index dad314a..9c8462b 100644 --- a/uv.lock +++ b/uv.lock @@ -431,7 +431,7 @@ requires-dist = [ { name = "falkordb", marker = "extra == 'falkordb'", specifier = ">=1.2" }, { name = "neo4j", marker = "extra == 'neo4j'", specifier = ">=5.20" }, { name = "oracledb", marker = "extra == 'oracle'", specifier = ">=3.0" }, - { name = "pi-pilgrim", specifier = ">=0.3.0" }, + { name = "pi-pilgrim", specifier = ">=0.4.0,<0.5" }, { name = "psycopg", extras = ["binary"], marker = "extra == 'postgres'", specifier = ">=3.2" }, { name = "psycopg-pool", marker = "extra == 'postgres'", specifier = ">=3.2" }, { name = "pydantic", specifier = ">=2.7" }, @@ -592,11 +592,11 @@ wheels = [ [[package]] name = "pi-pilgrim" -version = "0.3.0" +version = "0.4.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/b6/2a/70a3d7f0ef615009c9d9a47190ededb24e4317f4abdaed73bc305dc4ed20/pi_pilgrim-0.3.0.tar.gz", hash = "sha256:02ea03998ec38193f73264a93ba6f6f7dc2c78401bfeb87d38099484571145b3", size = 20845, upload-time = "2026-10-27T01:18:00.503Z" } +sdist = { url = "https://files.pythonhosted.org/packages/4f/cf/4f387a921d833cdcb87e264c1a6be5c11b194a901e665cb5284874dc900c/pi_pilgrim-0.4.0.tar.gz", hash = "sha256:e308a9e1e103ca48574ed86a48cf07e8bbaa5eb7c035382ff2b3dc6b7c85577d", size = 23656, upload-time = "2026-10-28T20:04:41.133Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/06/2f/dbc3bcf2ac3a32e2166e2c838d77a1adbd2c93e1e4b8ac5eb9b0d33d226a/pi_pilgrim-0.3.0-py3-none-any.whl", hash = "sha256:114e3b6ad18a514e38cdfd47c740e7010e653e93896c6e4289f40d40fffd5a06", size = 19588, upload-time = "2026-10-27T01:17:59.417Z" }, + { url = "https://files.pythonhosted.org/packages/08/77/212b07020427ac893273ffc71aa32ca1aab42da0556a0895ee75e2f82bd1/pi_pilgrim-0.4.0-py3-none-any.whl", hash = "sha256:de5f5d0010d56e7303a45e795a7bba4c505f226b6f7888db0600377d16c5a2a0", size = 21594, upload-time = "2026-10-28T20:04:39.792Z" }, ] [[package]]