From b4ade003f526882edcbda10a62fa75f2c2d7bb3f Mon Sep 17 00:00:00 2001 From: jasper blues Date: Wed, 28 Oct 2026 19:11:25 +1000 Subject: [PATCH 1/4] Bring back await DrivineAssembly.close(), 0.2.0's shutdown, so it closes the pools again 0.3.0 moved DrivineAssembly onto pi-pilgrim's lifecycle and dropped its async close(), so 0.2.0's documented shutdown, await assembly.drivine.close(), reached pi-pilgrim's sync close(), which refuses once the registry is made, and left the drivers and pools open. close() is back on the async assembly. It closes the assembly at once when nothing needs awaiting, and otherwise returns aclose() to await, so an application assembly's sync close(), which calls it without awaiting, still closes it. with DrivineAssembly() now refuses and points to async with. scripts/unasync.py leaves these out of the sync assembly, whose close() is pi-pilgrim's. The README says what changed from 0.2.0. Fixes #228 --- README.md | 19 ++++++- drivine/drivine_assembly.py | 38 ++++++++++++- scripts/unasync.py | 42 +++++++++++---- tests/unit/test_assembly_lifecycle.py | 78 +++++++++++++++++++++++++-- 4 files changed, 160 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index a6dd303..bd9c8d0 100644 --- a/README.md +++ b/README.md @@ -1049,8 +1049,9 @@ 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 sync assembly**, `drivine.sync.DrivineAssembly`, closes with `with` or `close()`. The async one closes with + `async with` or `await aclose()`, as its drivers close asynchronously; `await close()` still works, for code written + for 0.2.0, and `with` refuses, and says to use `async with`. - 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: @@ -1059,6 +1060,20 @@ 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 still closes + everything, in every release after 0.3.0 (0.3.0 itself raised `PilgrimError`). 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 `PilgrimError`: use `async with`, or `drivine.sync.DrivineAssembly` for the sync + API, which closes with `with` or `close()`. +- **Sync `close()` on your assembly** refuses once Drivine has made its registry, since the drivers close + asynchronously, and says to use `await aclose()`. + ## The sample app [`sample/`](sample) is the port of drivine4j's sample, as TypeScript's Drivine has it: an issue tracker and a guide's diff --git a/drivine/drivine_assembly.py b/drivine/drivine_assembly.py index 9563038..ee291f3 100644 --- a/drivine/drivine_assembly.py +++ b/drivine/drivine_assembly.py @@ -1,8 +1,9 @@ from __future__ import annotations -from typing import Self +from collections.abc import Awaitable, Generator +from typing import NoReturn, Self -from pipilgrim import PilgrimAssembly +from pipilgrim import PilgrimAssembly, PilgrimError from drivine.connection.database_registry import DEFAULT, DatabaseRegistry from drivine.dsl.index_advice_policy import IndexAdvicePolicy @@ -124,3 +125,36 @@ async def __aenter__(self) -> Self: await self.aclose() raise return self + + def close(self) -> Awaitable[None]: # pyright: ignore[reportIncompatibleMethodOverride] + """ + Closes the assembly, as `await aclose()` does: kept so that 0.2.0's shutdown, `await assembly.drivine.close()`, + still closes every database's driver and pool. Prefer `await aclose()`, or leaving `async with`. + + Await what it returns. It closes the assembly at once when nothing in it needs awaiting, as when the registry + was never made, so that an application assembly's sync `close()`, which calls it without awaiting, still + closes it; otherwise it returns `aclose()` to await. Closing twice does nothing. + """ + try: + super().close() + except PilgrimError: + # pi-pilgrim's close() refuses, before closing anything, when a teardown is async, as the registry's is. + return self.aclose() + return _Closed() + + def __enter__(self) -> NoReturn: + raise PilgrimError( + f"{type(self).__name__} closes its drivers and pools asynchronously: use async with, or " + f"drivine.sync.DrivineAssembly for the sync API." + ) + + def __exit__(self, *exc_info: object) -> NoReturn: + self.__enter__() + + +class _Closed: + """What `DrivineAssembly.close()` returns when it has already closed the assembly: awaiting it does nothing.""" + + def __await__(self) -> Generator[None, None, None]: + return + yield diff --git a/scripts/unasync.py b/scripts/unasync.py index 4a8801c..448b828 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 +are the definitions ASYNC_ONLY names. """ from __future__ import annotations @@ -48,6 +49,16 @@ class Tree: EXTERNAL = {"falkordb.asyncio": "falkordb"} """Driver modules whose sync counterpart is another module.""" +ASYNC_ONLY = { + "drivine.drivine_assembly": ( + "_Closed", + "DrivineAssembly.close", + "DrivineAssembly.__enter__", + "DrivineAssembly.__exit__", + ), +} +"""Definitions left out of the sync code, by module: the async API's own, which the sync API has from pi-pilgrim.""" + RENAMES = { "AsyncIterator": "Iterator", "AsyncIterable": "Iterable", @@ -137,16 +148,27 @@ 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, ()) + 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 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, "") return "".join(line for i, line in enumerate(lines) if i not in dropped) @@ -252,7 +274,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..26a2131 100644 --- a/tests/unit/test_assembly_lifecycle.py +++ b/tests/unit/test_assembly_lifecycle.py @@ -135,14 +135,59 @@ 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 + + +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(): + drivine = FakeDrivine() + with pytest.raises(PilgrimError, match="async with"): + with drivine: # type: ignore[misc] + pytest.fail("the body ran") + with pytest.raises(PilgrimError, match="async with"): + drivine.__exit__(None, None, None) + assert drivine.started == 0 + await drivine.aclose() class App(PilgrimAssembly): @@ -174,6 +219,33 @@ 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(): + # The application assembly's close() calls the DrivineAssembly's without awaiting it: nothing is left to await. + 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(PilgrimError, match="aclose"): + app.close() + assert app.drivine.provider.ended == 0 + + def test_a_sync_application_assembly_closes_the_drivine_it_holds(): with SyncApp() as app: app.greeter() From c6124385f92028ad5fe59f761d2fd3599bf265e0 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Wed, 28 Oct 2026 19:42:54 +1000 Subject: [PATCH 2/4] Re-raise refusals that aren't about an async teardown, let asyncio run what close() returns, keep the refusal text out of the sync copy, and say when close() must be awaited --- README.md | 11 ++- docs/design.md | 13 +-- drivine/drivine_assembly.py | 43 +++++++--- scripts/unasync.py | 25 +++++- tests/unit/test_assembly_lifecycle.py | 110 ++++++++++++++++++++++++++ tests/unit/test_unasync.py | 66 ++++++++++++++++ 6 files changed, 247 insertions(+), 21 deletions(-) create mode 100644 tests/unit/test_unasync.py diff --git a/README.md b/README.md index bd9c8d0..b7f00bc 100644 --- a/README.md +++ b/README.md @@ -1066,13 +1066,16 @@ registry with `async with DatabaseRegistry() as registry:`. 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 still closes - everything, in every release after 0.3.0 (0.3.0 itself raised `PilgrimError`). 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()`. + everything, in every release after 0.3.0 (0.3.0 itself raised `PilgrimError`, or `TypeError` when Drivine had made no + registry). Await it: called without `await`, it closes nothing once Drivine has made its registry, 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 `PilgrimError`: use `async with`, or `drivine.sync.DrivineAssembly` for the sync API, which closes with `with` or `close()`. - **Sync `close()` on your assembly** refuses once Drivine has made its registry, since the drivers close - asynchronously, and says to use `await aclose()`. + asynchronously, and says to use `await aclose()`. Hold Drivine with `AssemblyHolder.shared`, as above: an assembly + that holds it as a component, with `self.shared(DrivineAssembly)`, can't close it with sync `close()` at all. ## The sample app diff --git a/docs/design.md b/docs/design.md index 0bf7ebd..a8ce64d 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. `DrivineAssembly.close()`, kept for 0.2.0's `await drivine.close()`, is sync and returns something to await: + `aclose()` once the registry needs closing, else a finished coroutine, so an application assembly's sync `close()` + still closes a `DrivineAssembly` with no registry. `with` on it refuses, as its exit couldn't await. 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 ee291f3..0bbca09 100644 --- a/drivine/drivine_assembly.py +++ b/drivine/drivine_assembly.py @@ -1,7 +1,7 @@ from __future__ import annotations -from collections.abc import Awaitable, Generator -from typing import NoReturn, Self +from collections.abc import Coroutine, Generator +from typing import Any, NoReturn, Self from pipilgrim import PilgrimAssembly, PilgrimError @@ -126,19 +126,24 @@ async def __aenter__(self) -> Self: raise return self - def close(self) -> Awaitable[None]: # pyright: ignore[reportIncompatibleMethodOverride] + def close(self) -> Coroutine[Any, Any, None]: # pyright: ignore[reportIncompatibleMethodOverride] """ Closes the assembly, as `await aclose()` does: kept so that 0.2.0's shutdown, `await assembly.drivine.close()`, still closes every database's driver and pool. Prefer `await aclose()`, or leaving `async with`. - Await what it returns. It closes the assembly at once when nothing in it needs awaiting, as when the registry - was never made, so that an application assembly's sync `close()`, which calls it without awaiting, still - closes it; otherwise it returns `aclose()` to await. Closing twice does nothing. + Await what it returns: left unawaited, it closes nothing once the registry is made, and Python warns that a + coroutine was never awaited. It can't refuse a call from sync code instead, as `asyncio.run(drivine.close())` + calls it before any loop runs. It closes the assembly at once when nothing in it needs awaiting, as when the + registry was never made, so that an application assembly's sync `close()`, which calls it without awaiting, + still closes it; otherwise it returns `aclose()` to await. Closing twice does nothing. """ try: super().close() - except PilgrimError: + except PilgrimError as error: # pi-pilgrim's close() refuses, before closing anything, when a teardown is async, as the registry's is. + # Any other refusal, such as closing while making a component, is raised. + if _ASYNC_TEARDOWN_REFUSAL not in str(error): + raise return self.aclose() return _Closed() @@ -152,9 +157,29 @@ def __exit__(self, *exc_info: object) -> NoReturn: self.__enter__() -class _Closed: - """What `DrivineAssembly.close()` returns when it has already closed the assembly: awaiting it does nothing.""" +_ASYNC_TEARDOWN_REFUSAL = "is async. Close it with await aclose()." +""" +How pi-pilgrim's close() says a teardown is async, in `_refuse_async` (pipilgrim/pilgrim_assembly.py:586 in 0.3.0 and +0.3.1), as it raises no subclass of PilgrimError for it: liberation-data/pi-pilgrim#9. A test fails if the text changes. +""" + + +class _Closed(Coroutine[Any, Any, None]): + """ + What `DrivineAssembly.close()` returns when it has already closed the assembly: awaiting it does nothing. A + Coroutine, so that `asyncio.run` and `create_task` take it as they take `aclose()`, but not a native one, so that + leaving it unawaited, as an application assembly's sync `close()` does, raises no warning. + """ def __await__(self) -> Generator[None, None, None]: return yield + + def send(self, value: object) -> NoReturn: + raise StopIteration + + def throw(self, typ: Any, val: Any = None, tb: Any = None) -> NoReturn: + raise typ if val is None else val + + def close(self) -> None: + pass diff --git a/scripts/unasync.py b/scripts/unasync.py index 448b828..442243c 100644 --- a/scripts/unasync.py +++ b/scripts/unasync.py @@ -51,13 +51,17 @@ class Tree: ASYNC_ONLY = { "drivine.drivine_assembly": ( + "_ASYNC_TEARDOWN_REFUSAL", "_Closed", "DrivineAssembly.close", "DrivineAssembly.__enter__", "DrivineAssembly.__exit__", ), } -"""Definitions left out of the sync code, by module: the async API's own, which the sync API has from pi-pilgrim.""" +""" +Definitions, and assignments to a name, left out of the sync code, by module: the async API's own, which the sync API +has from pi-pilgrim. +""" RENAMES = { "AsyncIterator": "Iterator", @@ -153,13 +157,26 @@ def without_async_only(text: str, source: ast.Module, module: str = "") -> str: lines = text.splitlines(keepends=True) dropped: set[int] = set() 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: + for i, it in enumerate(body): + if isinstance(it, ast.Assign) and len(it.targets) == 1 and isinstance(it.targets[0], ast.Name): + name = f"{prefix}{it.targets[0].id}" + if name in named: + found.add(name) + # With the docstring that follows it, if it has one. + after = body[i + 1] if i + 1 < len(body) else None + docstring = isinstance(after, ast.Expr) and isinstance(after.value, ast.Constant) + last = after if docstring and after else it + dropped.update(range(it.lineno - 1, last.end_lineno or last.lineno)) + continue 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 ): @@ -169,6 +186,10 @@ def visit(body: list[ast.stmt], prefix: str) -> None: 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) diff --git a/tests/unit/test_assembly_lifecycle.py b/tests/unit/test_assembly_lifecycle.py index 26a2131..0c30695 100644 --- a/tests/unit/test_assembly_lifecycle.py +++ b/tests/unit/test_assembly_lifecycle.py @@ -3,12 +3,16 @@ closes drivine4j's with its context; so does a DatabaseRegistry that was entered. Async and sync alike. """ +import asyncio +import gc +import inspect from collections.abc import Iterator import pytest from pipilgrim import AssemblyHolder, PilgrimAssembly, PilgrimError from drivine import DatabaseRegistry, DatabaseType, DrivineAssembly +from drivine.drivine_assembly import _ASYNC_TEARDOWN_REFUSAL from drivine.sync import ConnectionProvider as SyncConnectionProvider from drivine.sync import DatabaseRegistry as SyncDatabaseRegistry from drivine.sync import DrivineAssembly as SyncDrivineAssembly @@ -171,6 +175,74 @@ async def test_await_close_after_aclose_does_nothing(): 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_what_close_returns_once_closed_is_a_finished_coroutine(): + closed = FakeDrivine().close() + assert asyncio.iscoroutine(closed) and not inspect.iscoroutine(closed) + with pytest.raises(StopIteration): + closed.send(None) + with pytest.raises(ValueError, match="thrown"): + closed.throw(ValueError("thrown")) + with pytest.raises(ValueError): + closed.throw(ValueError) + with pytest.raises(KeyError, match="value"): + closed.throw(KeyError, KeyError("value")) + closed.close() + closed.close() + + +def test_close_not_awaited_leaves_the_providers_open_and_python_warns(): + drivine = FakeDrivine() + drivine.registry() + with pytest.warns(RuntimeWarning, match="never awaited"): + drivine.close() + gc.collect() + assert drivine.provider.ended == 0 + asyncio.run(drivine.aclose()) + assert drivine.provider.ended == 1 + + +def test_pi_pilgrim_still_refuses_an_async_teardown_with_the_text_close_matches(): + # close() tells this refusal from others by its text. If pi-pilgrim rewords it, close() would raise it again, as + # 0.3.0 did (#228): this fails first. + class AsyncTeardown(PilgrimAssembly): + def registry(self) -> DatabaseRegistry: + return self.shared(DatabaseRegistry) + + assembly = AsyncTeardown() + assembly.registry() + with pytest.raises(PilgrimError) as refused: + assembly.close() + assert _ASYNC_TEARDOWN_REFUSAL in str(refused.value) + asyncio.run(assembly.aclose()) + + +def test_a_sync_close_of_an_assembly_holding_drivine_as_a_component_fails_but_closes_it(): + # pi-pilgrim's sync close() refuses what a component's close() returns when it is awaitable, as DrivineAssembly's + # is. Hold Drivine with AssemblyHolder.shared, as App does, or close the holder with await aclose(). + class Holder(PilgrimAssembly): + def drivine(self) -> FakeDrivine: + return self.shared(FakeDrivine) + + holder = Holder() + drivine = holder.drivine() + with pytest.raises(ExceptionGroup) as failed: + holder.close() + assert "await aclose()" in str(failed.value.exceptions[0]) + with pytest.raises(PilgrimError, match="is closed"): + drivine.registry() + + def test_the_sync_assembly_closes_with_close(): drivine = SyncFakeDrivine() drivine.registry() @@ -190,6 +262,44 @@ async def test_the_async_assembly_refuses_with(): await drivine.aclose() +async def test_close_raises_a_refusal_that_is_not_about_an_async_teardown(): + # Closing from inside a factory is refused for another reason: close() raises it, not aclose() to await later. + class ClosesWhileMaking(FakeDrivine): + def closer(self) -> object: + return self.shared(lambda: self.close()) + + drivine = ClosesWhileMaking() + drivine.registry() + with pytest.raises(PilgrimError, match="while it is making a component"): + drivine.closer() + assert drivine.provider.ended == 0 + await drivine.close() + assert drivine.provider.ended == 1 + + +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): def __init__(self) -> None: self.drivine = AssemblyHolder.shared(FakeDrivine) diff --git a/tests/unit/test_unasync.py b/tests/unit/test_unasync.py new file mode 100644 index 0000000..2c512bf --- /dev/null +++ b/tests/unit/test_unasync.py @@ -0,0 +1,66 @@ +"""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 + + +REFUSAL = "await it" +'''Its docstring.''' + +KEPT = 6 +""" + + +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 "REFUSAL" in text + 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") + + +def test_async_only_leaves_out_an_assignment_with_its_docstring(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(unasync, "ASYNC_ONLY", {"m": ("REFUSAL",)}) + text = unasync.without_async_only(SOURCE, ast.parse(SOURCE), "m") + assert "REFUSAL" not in text and "Its docstring" not in text + assert "KEPT = 6" in text and "return 5" in text From 5772497f99e92cc3ce8dcaa26762ed101765f4a4 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Thu, 29 Oct 2026 06:30:14 +1000 Subject: [PATCH 3/4] Make DrivineAssembly pi-pilgrim 0.4's AsyncPilgrimAssembly, so await close() closes it and so does a holder's sync close pi-pilgrim 0.4.0 has an AsyncPilgrimAssembly whose close() is aclose(), and closes a held or owned assembly through its own lifecycle. On it, DrivineAssembly drops its sync close() override, the finished coroutine it returned, and the match on pi-pilgrim's refusal text. An assembly holding Drivine as a component, self.shared(DrivineAssembly), now closes it with a sync close() while it has no registry, where it raised an ExceptionGroup. A refusal is now AsyncTeardownRequired. __enter__ stays, to name the sync API in its refusal, so unasync's ASYNC_ONLY keeps only it; the sync DrivineAssembly is unchanged. --- docs/09-with-pi-pilgrim.md | 32 +++---- docs/design.md | 6 +- drivine/drivine_assembly.py | 62 ++------------ pyproject.toml | 2 +- scripts/unasync.py | 29 ++----- tests/unit/test_assembly_lifecycle.py | 115 +++++++++++++------------- tests/unit/test_unasync.py | 14 ---- uv.lock | 10 +-- 8 files changed, 94 insertions(+), 176 deletions(-) diff --git a/docs/09-with-pi-pilgrim.md b/docs/09-with-pi-pilgrim.md index 40d3ff2..ee6fd49 100644 --- a/docs/09-with-pi-pilgrim.md +++ b/docs/09-with-pi-pilgrim.md @@ -49,9 +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 with - `async with` or `await aclose()`, as its drivers close asynchronously; `await close()` still works, for code written - for 0.2.0, and `with` refuses, and says to use `async with`. +- **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: @@ -65,14 +68,15 @@ registry with `async with DatabaseRegistry() as registry:`. 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 still closes - everything, in every release after 0.3.0 (0.3.0 itself raised `PilgrimError`, or `TypeError` when Drivine had made no - registry). Await it: called without `await`, it closes nothing once Drivine has made its registry, 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 `PilgrimError`: use `async with`, or `drivine.sync.DrivineAssembly` for the sync - API, which closes with `with` or `close()`. -- **Sync `close()` on your assembly** refuses once Drivine has made its registry, since the drivers close - asynchronously, and says to use `await aclose()`. Hold Drivine with `AssemblyHolder.shared`, as above: an assembly - that holds it as a component, with `self.shared(DrivineAssembly)`, can't close it with sync `close()` at all. +- **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()`. diff --git a/docs/design.md b/docs/design.md index a8ce64d..87ec346 100644 --- a/docs/design.md +++ b/docs/design.md @@ -565,9 +565,9 @@ class AppAssembly(PilgrimAssembly): - 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.close()`, kept for 0.2.0's `await drivine.close()`, is sync and returns something to await: - `aclose()` once the registry needs closing, else a finished coroutine, so an application assembly's sync `close()` - still closes a `DrivineAssembly` with no registry. `with` on it refuses, as its exit couldn't await. Entering + 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. diff --git a/drivine/drivine_assembly.py b/drivine/drivine_assembly.py index 0bbca09..bd33420 100644 --- a/drivine/drivine_assembly.py +++ b/drivine/drivine_assembly.py @@ -1,9 +1,8 @@ from __future__ import annotations -from collections.abc import Coroutine, Generator -from typing import Any, NoReturn, Self +from typing import NoReturn, Self -from pipilgrim import PilgrimAssembly, PilgrimError +from pipilgrim import AsyncPilgrimAssembly, AsyncTeardownRequired from drivine.connection.database_registry import DEFAULT, DatabaseRegistry from drivine.dsl.index_advice_policy import IndexAdvicePolicy @@ -16,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: @@ -126,60 +125,9 @@ async def __aenter__(self) -> Self: raise return self - def close(self) -> Coroutine[Any, Any, None]: # pyright: ignore[reportIncompatibleMethodOverride] - """ - Closes the assembly, as `await aclose()` does: kept so that 0.2.0's shutdown, `await assembly.drivine.close()`, - still closes every database's driver and pool. Prefer `await aclose()`, or leaving `async with`. - - Await what it returns: left unawaited, it closes nothing once the registry is made, and Python warns that a - coroutine was never awaited. It can't refuse a call from sync code instead, as `asyncio.run(drivine.close())` - calls it before any loop runs. It closes the assembly at once when nothing in it needs awaiting, as when the - registry was never made, so that an application assembly's sync `close()`, which calls it without awaiting, - still closes it; otherwise it returns `aclose()` to await. Closing twice does nothing. - """ - try: - super().close() - except PilgrimError as error: - # pi-pilgrim's close() refuses, before closing anything, when a teardown is async, as the registry's is. - # Any other refusal, such as closing while making a component, is raised. - if _ASYNC_TEARDOWN_REFUSAL not in str(error): - raise - return self.aclose() - return _Closed() - def __enter__(self) -> NoReturn: - raise PilgrimError( + """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." ) - - def __exit__(self, *exc_info: object) -> NoReturn: - self.__enter__() - - -_ASYNC_TEARDOWN_REFUSAL = "is async. Close it with await aclose()." -""" -How pi-pilgrim's close() says a teardown is async, in `_refuse_async` (pipilgrim/pilgrim_assembly.py:586 in 0.3.0 and -0.3.1), as it raises no subclass of PilgrimError for it: liberation-data/pi-pilgrim#9. A test fails if the text changes. -""" - - -class _Closed(Coroutine[Any, Any, None]): - """ - What `DrivineAssembly.close()` returns when it has already closed the assembly: awaiting it does nothing. A - Coroutine, so that `asyncio.run` and `create_task` take it as they take `aclose()`, but not a native one, so that - leaving it unawaited, as an application assembly's sync `close()` does, raises no warning. - """ - - def __await__(self) -> Generator[None, None, None]: - return - yield - - def send(self, value: object) -> NoReturn: - raise StopIteration - - def throw(self, typ: Any, val: Any = None, tb: Any = None) -> NoReturn: - raise typ if val is None else val - - def close(self) -> None: - pass 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 442243c..4ce4e81 100644 --- a/scripts/unasync.py +++ b/scripts/unasync.py @@ -9,7 +9,7 @@ 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, as -are the definitions ASYNC_ONLY names. +is a definition ASYNC_ONLY names. """ from __future__ import annotations @@ -49,18 +49,10 @@ class Tree: EXTERNAL = {"falkordb.asyncio": "falkordb"} """Driver modules whose sync counterpart is another module.""" -ASYNC_ONLY = { - "drivine.drivine_assembly": ( - "_ASYNC_TEARDOWN_REFUSAL", - "_Closed", - "DrivineAssembly.close", - "DrivineAssembly.__enter__", - "DrivineAssembly.__exit__", - ), -} +ASYNC_ONLY = {"drivine.drivine_assembly": ("DrivineAssembly.__enter__",)} """ -Definitions, and assignments to a name, left out of the sync code, by module: the async API's own, which the sync API -has from pi-pilgrim. +Definitions left out of the sync code, by module: the async API's own. The async DrivineAssembly refuses `with`, naming +the sync API; the sync one, a plain PilgrimAssembly, is closed by it. """ RENAMES = { @@ -88,6 +80,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.""" @@ -161,17 +154,7 @@ def without_async_only(text: str, source: ast.Module, module: str = "") -> str: definitions = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef) def visit(body: list[ast.stmt], prefix: str) -> None: - for i, it in enumerate(body): - if isinstance(it, ast.Assign) and len(it.targets) == 1 and isinstance(it.targets[0], ast.Name): - name = f"{prefix}{it.targets[0].id}" - if name in named: - found.add(name) - # With the docstring that follows it, if it has one. - after = body[i + 1] if i + 1 < len(body) else None - docstring = isinstance(after, ast.Expr) and isinstance(after.value, ast.Constant) - last = after if docstring and after else it - dropped.update(range(it.lineno - 1, last.end_lineno or last.lineno)) - continue + for it in body: if not isinstance(it, definitions): continue name = f"{prefix}{it.name}" diff --git a/tests/unit/test_assembly_lifecycle.py b/tests/unit/test_assembly_lifecycle.py index 0c30695..f993315 100644 --- a/tests/unit/test_assembly_lifecycle.py +++ b/tests/unit/test_assembly_lifecycle.py @@ -5,14 +5,12 @@ import asyncio import gc -import inspect 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.drivine_assembly import _ASYNC_TEARDOWN_REFUSAL from drivine.sync import ConnectionProvider as SyncConnectionProvider from drivine.sync import DatabaseRegistry as SyncDatabaseRegistry from drivine.sync import DrivineAssembly as SyncDrivineAssembly @@ -186,59 +184,62 @@ def test_asyncio_run_takes_what_close_returns(registry: bool): drivine.registry() -def test_what_close_returns_once_closed_is_a_finished_coroutine(): - closed = FakeDrivine().close() - assert asyncio.iscoroutine(closed) and not inspect.iscoroutine(closed) - with pytest.raises(StopIteration): - closed.send(None) - with pytest.raises(ValueError, match="thrown"): - closed.throw(ValueError("thrown")) - with pytest.raises(ValueError): - closed.throw(ValueError) - with pytest.raises(KeyError, match="value"): - closed.throw(KeyError, KeyError("value")) - closed.close() - closed.close() +def test_drivine_is_an_async_pilgrim_assembly_and_the_sync_one_a_plain_one(): + assert issubclass(DrivineAssembly, AsyncPilgrimAssembly) + assert not issubclass(SyncDrivineAssembly, AsyncPilgrimAssembly) -def test_close_not_awaited_leaves_the_providers_open_and_python_warns(): +@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() - drivine.registry() + 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 -def test_pi_pilgrim_still_refuses_an_async_teardown_with_the_text_close_matches(): - # close() tells this refusal from others by its text. If pi-pilgrim rewords it, close() would raise it again, as - # 0.3.0 did (#228): this fails first. - class AsyncTeardown(PilgrimAssembly): - def registry(self) -> DatabaseRegistry: - return self.shared(DatabaseRegistry) +class Holder(PilgrimAssembly): + """Holds Drivine as a component, rather than with AssemblyHolder.shared.""" - assembly = AsyncTeardown() - assembly.registry() - with pytest.raises(PilgrimError) as refused: - assembly.close() - assert _ASYNC_TEARDOWN_REFUSAL in str(refused.value) - asyncio.run(assembly.aclose()) + def drivine(self) -> FakeDrivine: + return self.shared(FakeDrivine) -def test_a_sync_close_of_an_assembly_holding_drivine_as_a_component_fails_but_closes_it(): - # pi-pilgrim's sync close() refuses what a component's close() returns when it is awaitable, as DrivineAssembly's - # is. Hold Drivine with AssemblyHolder.shared, as App does, or close the holder with await aclose(). - class Holder(PilgrimAssembly): - 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() - with pytest.raises(ExceptionGroup) as failed: + drivine.registry() + with pytest.raises(AsyncTeardownRequired, match="aclose"): holder.close() - assert "await aclose()" in str(failed.value.exceptions[0]) + # 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() @@ -251,32 +252,15 @@ def test_the_sync_assembly_closes_with_close(): assert drivine.provider.ended == 1 -async def test_the_async_assembly_refuses_with(): +async def test_the_async_assembly_refuses_with_and_names_the_sync_api(): drivine = FakeDrivine() - with pytest.raises(PilgrimError, match="async with"): + with pytest.raises(AsyncTeardownRequired, match=r"async with, or drivine\.sync\.DrivineAssembly"): with drivine: # type: ignore[misc] pytest.fail("the body ran") - with pytest.raises(PilgrimError, match="async with"): - drivine.__exit__(None, None, None) assert drivine.started == 0 await drivine.aclose() -async def test_close_raises_a_refusal_that_is_not_about_an_async_teardown(): - # Closing from inside a factory is refused for another reason: close() raises it, not aclose() to await later. - class ClosesWhileMaking(FakeDrivine): - def closer(self) -> object: - return self.shared(lambda: self.close()) - - drivine = ClosesWhileMaking() - drivine.registry() - with pytest.raises(PilgrimError, match="while it is making a component"): - drivine.closer() - assert drivine.provider.ended == 0 - await drivine.close() - assert drivine.provider.ended == 1 - - async def test_await_close_ends_every_provider_when_one_fails(): class FailingProvider(EndingProvider): async def end(self) -> None: @@ -340,7 +324,7 @@ async def test_await_close_on_an_application_assemblys_drivine_closes_it_as_in_0 def test_a_sync_close_of_an_application_assembly_closes_a_drivine_with_no_registry(): - # The application assembly's close() calls the DrivineAssembly's without awaiting it: nothing is left to await. + # 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() @@ -351,9 +335,22 @@ def test_a_sync_close_of_an_application_assembly_closes_a_drivine_with_no_regist def test_a_sync_close_of_an_application_assembly_refuses_once_drivine_has_a_registry(): app = App() app.greeter() - with pytest.raises(PilgrimError, match="aclose"): + with pytest.raises(AsyncTeardownRequired, match="aclose"): app.close() assert app.drivine.provider.ended == 0 + asyncio.run(app.aclose()) + assert app.drivine.provider.ended == 1 + + +@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(): diff --git a/tests/unit/test_unasync.py b/tests/unit/test_unasync.py index 2c512bf..fc44c55 100644 --- a/tests/unit/test_unasync.py +++ b/tests/unit/test_unasync.py @@ -35,12 +35,6 @@ def keep(self): def close(): return 5 - - -REFUSAL = "await it" -'''Its docstring.''' - -KEPT = 6 """ @@ -49,7 +43,6 @@ def test_async_only_leaves_out_exactly_the_named_definitions(monkeypatch: pytest 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 "REFUSAL" in text assert unasync.without_async_only(SOURCE, ast.parse(SOURCE), "other") == SOURCE @@ -57,10 +50,3 @@ def test_async_only_refuses_a_name_the_module_does_not_define(monkeypatch: pytes monkeypatch.setattr(unasync, "ASYNC_ONLY", {"m": ("Assembly.closed",)}) with pytest.raises(SystemExit, match="Assembly.closed"): unasync.without_async_only(SOURCE, ast.parse(SOURCE), "m") - - -def test_async_only_leaves_out_an_assignment_with_its_docstring(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(unasync, "ASYNC_ONLY", {"m": ("REFUSAL",)}) - text = unasync.without_async_only(SOURCE, ast.parse(SOURCE), "m") - assert "REFUSAL" not in text and "Its docstring" not in text - assert "KEPT = 6" in text and "return 5" in text diff --git a/uv.lock b/uv.lock index dad314a..aa50630 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 5 +revision = 3 requires-python = ">=3.11" [[package]] @@ -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]] From 2340dcdfcf615345e62943cc694e9159963b8bd5 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Thu, 29 Oct 2026 06:37:37 +1000 Subject: [PATCH 4/4] Test a subclass that overrides aclose(), say its sync close refuses from the start, check the sync assembly is a plain one, and keep uv.lock's revision --- docs/09-with-pi-pilgrim.md | 11 +++++----- scripts/unasync.py | 5 +++-- tests/unit/test_assembly_lifecycle.py | 30 +++++++++++++++++++++++++++ uv.lock | 2 +- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/docs/09-with-pi-pilgrim.md b/docs/09-with-pi-pilgrim.md index ee6fd49..b79c7e1 100644 --- a/docs/09-with-pi-pilgrim.md +++ b/docs/09-with-pi-pilgrim.md @@ -70,13 +70,14 @@ the application assembly that holds it closes, and closing it closes every datab - **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()`. + 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()`. + `await aclose()`. It refuses from the start if your subclass of `DrivineAssembly` overrides `aclose()`, which a sync + close can't run. diff --git a/scripts/unasync.py b/scripts/unasync.py index 4ce4e81..e1e6850 100644 --- a/scripts/unasync.py +++ b/scripts/unasync.py @@ -51,8 +51,9 @@ class Tree: ASYNC_ONLY = {"drivine.drivine_assembly": ("DrivineAssembly.__enter__",)} """ -Definitions left out of the sync code, by module: the async API's own. The async DrivineAssembly refuses `with`, naming -the sync API; the sync one, a plain PilgrimAssembly, is closed by it. +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 = { diff --git a/tests/unit/test_assembly_lifecycle.py b/tests/unit/test_assembly_lifecycle.py index f993315..a139524 100644 --- a/tests/unit/test_assembly_lifecycle.py +++ b/tests/unit/test_assembly_lifecycle.py @@ -186,6 +186,7 @@ def test_asyncio_run_takes_what_close_returns(registry: bool): 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) @@ -342,6 +343,35 @@ def test_a_sync_close_of_an_application_assembly_refuses_once_drivine_has_a_regi 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) diff --git a/uv.lock b/uv.lock index aa50630..9c8462b 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 5 requires-python = ">=3.11" [[package]]