diff --git a/CHANGELOG.md b/CHANGELOG.md index 2840b08..1e7ea9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,30 @@ versioning follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- `py-common[migrations]` could not import `py_common.persistence.migrations`. It + failed with *"The SQLAlchemy asyncio module requires that the Python 'greenlet' + library is installed"* — because importing that submodule runs + `persistence/__init__.py`, which imported `engine` and therefore + `sqlalchemy.ext.asyncio`, whose `greenlet` dependency only the `persistence` + extra's `sqlalchemy[asyncio]` brings. The Alembic helpers are synchronous and + need none of it. + + `persistence/__init__.py` now resolves its re-exports on first access, like + `runtime` and `http` since 0.2.1. No public name moved. + + It went unnoticed because whether it fails depends on what a fresh resolve + happens to pull in; CI stayed green while a local run went red. + `scripts/check-extras-isolation.sh` resolves fresh, which is how it surfaced. + +- `scripts/check-extras-isolation.sh` now also imports `py_common.testing.routes` + (under `http`) and `py_common.testing.tokens` (under `security`). They are + public API a consumer imports in its own test suite and each carries its own + extra, so leaving them out left a hole exactly where a helper is most likely to + reach across packages. + + ## [0.2.1] - 2026-09-18 ### Fixed diff --git a/scripts/check-extras-isolation.sh b/scripts/check-extras-isolation.sh index 8cb6e9a..0af0199 100755 --- a/scripts/check-extras-isolation.sh +++ b/scripts/check-extras-isolation.sh @@ -14,9 +14,13 @@ set -euo pipefail # extra:module[,module...] -- what installing that extra alone must give you. +# The py_common.testing.* submodules are listed because they are public API a +# consumer imports in its own test suite, and they carry their own extra: routes +# needs http, tokens needs security. Leaving them out left a hole exactly where +# a helper is most likely to reach across packages. CASES=( - "http:py_common.http,py_common.http.middleware" - "security:py_common.security" + "http:py_common.http,py_common.http.middleware,py_common.testing.routes" + "security:py_common.security,py_common.testing.tokens" "cache:py_common.cache" "storage:py_common.storage" "telemetry:py_common.telemetry" diff --git a/src/py_common/persistence/__init__.py b/src/py_common/persistence/__init__.py index 65b9673..88452c8 100644 --- a/src/py_common/persistence/__init__.py +++ b/src/py_common/persistence/__init__.py @@ -1,48 +1,97 @@ -"""Persistence abstractions: engine factory, repository interface, and unit of work.""" - -from py_common.persistence.base import NAMING_CONVENTION, Base, metadata -from py_common.persistence.engine import ( - create_engine_and_sessionmaker, - database_lifespan_resource, -) -from py_common.persistence.migrations import ( - build_alembic_config, - current_revision, - downgrade, - migration_lifespan_resource, - upgrade_to_head, -) -from py_common.persistence.mixins import ( - SoftDeleteMixin, - TimestampMixin, - UUIDv7PrimaryKeyMixin, -) -from py_common.persistence.pagination import paginate_cursor, paginate_offset -from py_common.persistence.query_logging import install_query_logger -from py_common.persistence.repository import Repository -from py_common.persistence.sqlalchemy_repository import SqlAlchemyRepository -from py_common.persistence.sqlalchemy_uow import SqlAlchemyUnitOfWork -from py_common.persistence.unit_of_work import UnitOfWork - -__all__ = [ - "NAMING_CONVENTION", - "Base", - "Repository", - "SoftDeleteMixin", - "SqlAlchemyRepository", - "SqlAlchemyUnitOfWork", - "TimestampMixin", - "UUIDv7PrimaryKeyMixin", - "UnitOfWork", - "build_alembic_config", - "create_engine_and_sessionmaker", - "current_revision", - "database_lifespan_resource", - "downgrade", - "install_query_logger", - "metadata", - "migration_lifespan_resource", - "paginate_cursor", - "paginate_offset", - "upgrade_to_head", -] +"""Persistence abstractions: engine factory, repository interface, and unit of work. + +Re-exports resolve lazily, for the reason given in :mod:`py_common.runtime`: +Python executes this file whenever anything imports one of its submodules, so +eager re-exports charge every importer for the union of this package's +dependencies. + +The concrete failure this fixes: ``py_common.persistence.migrations`` promises to +work on the ``migrations`` extra (Alembic + psycopg, both synchronous), but +importing it ran this file, which imported ``engine`` and therefore +``sqlalchemy.ext.asyncio`` — and that needs ``greenlet``, which only the +``persistence`` extra's ``sqlalchemy[asyncio]`` brings. `[migrations]` alone +raised *"The SQLAlchemy asyncio module requires that the Python 'greenlet' library +is installed"*. It went unnoticed because it depends on what a fresh resolve +happens to pull in; `scripts/check-extras-isolation.sh` resolves fresh, which is +how it surfaced. + +``from py_common.persistence import Repository`` is unchanged; the submodule loads +the first time the name is read. +""" + +from __future__ import annotations + +from importlib import import_module +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from py_common.persistence.base import NAMING_CONVENTION as NAMING_CONVENTION + from py_common.persistence.base import Base as Base + from py_common.persistence.base import metadata as metadata + from py_common.persistence.engine import ( + create_engine_and_sessionmaker as create_engine_and_sessionmaker, + ) + from py_common.persistence.engine import ( + database_lifespan_resource as database_lifespan_resource, + ) + from py_common.persistence.migrations import build_alembic_config as build_alembic_config + from py_common.persistence.migrations import current_revision as current_revision + from py_common.persistence.migrations import downgrade as downgrade + from py_common.persistence.migrations import ( + migration_lifespan_resource as migration_lifespan_resource, + ) + from py_common.persistence.migrations import upgrade_to_head as upgrade_to_head + from py_common.persistence.mixins import SoftDeleteMixin as SoftDeleteMixin + from py_common.persistence.mixins import TimestampMixin as TimestampMixin + from py_common.persistence.mixins import UUIDv7PrimaryKeyMixin as UUIDv7PrimaryKeyMixin + from py_common.persistence.pagination import paginate_cursor as paginate_cursor + from py_common.persistence.pagination import paginate_offset as paginate_offset + from py_common.persistence.query_logging import install_query_logger as install_query_logger + from py_common.persistence.repository import Repository as Repository + from py_common.persistence.sqlalchemy_repository import ( + SqlAlchemyRepository as SqlAlchemyRepository, + ) + from py_common.persistence.sqlalchemy_uow import SqlAlchemyUnitOfWork as SqlAlchemyUnitOfWork + from py_common.persistence.unit_of_work import UnitOfWork as UnitOfWork + +# name -> defining submodule. Grouped so the cost of each is visible: `base`, +# `mixins`, `repository` and `unit_of_work` are plain SQLAlchemy Core or ABCs; +# `engine`, `query_logging`, `sqlalchemy_repository` and `sqlalchemy_uow` need the +# asyncio stack; `migrations` needs Alembic; `pagination` reaches into +# `py_common.http` for the cursor codec. +_LAZY: dict[str, str] = { + "NAMING_CONVENTION": "py_common.persistence.base", + "Base": "py_common.persistence.base", + "metadata": "py_common.persistence.base", + "create_engine_and_sessionmaker": "py_common.persistence.engine", + "database_lifespan_resource": "py_common.persistence.engine", + "build_alembic_config": "py_common.persistence.migrations", + "current_revision": "py_common.persistence.migrations", + "downgrade": "py_common.persistence.migrations", + "migration_lifespan_resource": "py_common.persistence.migrations", + "upgrade_to_head": "py_common.persistence.migrations", + "SoftDeleteMixin": "py_common.persistence.mixins", + "TimestampMixin": "py_common.persistence.mixins", + "UUIDv7PrimaryKeyMixin": "py_common.persistence.mixins", + "paginate_cursor": "py_common.persistence.pagination", + "paginate_offset": "py_common.persistence.pagination", + "install_query_logger": "py_common.persistence.query_logging", + "Repository": "py_common.persistence.repository", + "SqlAlchemyRepository": "py_common.persistence.sqlalchemy_repository", + "SqlAlchemyUnitOfWork": "py_common.persistence.sqlalchemy_uow", + "UnitOfWork": "py_common.persistence.unit_of_work", +} + +__all__ = sorted(_LAZY) + + +def __getattr__(name: str) -> Any: + """PEP 562 hook: resolve a re-export on first access.""" + module = _LAZY.get(name) + if module is None: + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + return getattr(import_module(module), name) + + +def __dir__() -> list[str]: + return __all__ diff --git a/tests/test_lazy_exports.py b/tests/test_lazy_exports.py new file mode 100644 index 0000000..e45fef8 --- /dev/null +++ b/tests/test_lazy_exports.py @@ -0,0 +1,76 @@ +"""The three packages whose ``__init__`` resolves re-exports on first access. + +`runtime`, `http` and `persistence` re-export lazily so that importing one of +their submodules does not charge the caller for the whole package's dependency +set — the bug that made four extras unimportable in 0.2.0 and `[migrations]` +unimportable after it. Nothing tested the mechanism itself until now: the 0.2.1 +fix was verified only through `scripts/check-extras-isolation.sh`, which needs a +throwaway environment per extra and therefore cannot run in this suite. + +These tests are cheap and cover the contract the mechanism has to keep. +""" + +from __future__ import annotations + +import importlib +import subprocess +import sys + +import pytest + +LAZY_PACKAGES = ["py_common.runtime", "py_common.http", "py_common.persistence"] + +# package -> a submodule that costs real dependencies, which importing the +# package must therefore *not* pull in. +HEAVY_SUBMODULE = { + "py_common.runtime": "py_common.runtime.app", + "py_common.http": "py_common.http.client", + "py_common.persistence": "py_common.persistence.engine", +} + + +@pytest.mark.parametrize("package", LAZY_PACKAGES) +def test_every_exported_name_resolves(package: str) -> None: + """``__all__`` is a promise; a typo in the lazy map would break it silently.""" + module = importlib.import_module(package) + + for name in module.__all__: + assert getattr(module, name) is not None + + +@pytest.mark.parametrize("package", LAZY_PACKAGES) +def test_an_unknown_name_raises_attribute_error(package: str) -> None: + """``__getattr__`` must not swallow a genuine typo into an import error.""" + module = importlib.import_module(package) + + with pytest.raises(AttributeError, match="no attribute 'nope'"): + _ = module.nope # type: ignore[attr-defined] + + +@pytest.mark.parametrize("package", LAZY_PACKAGES) +def test_dir_matches_all(package: str) -> None: + """Otherwise tab-completion and `dir()` disagree with the documented surface.""" + module = importlib.import_module(package) + + assert dir(module) == sorted(module.__all__) + + +@pytest.mark.parametrize("package", LAZY_PACKAGES) +def test_importing_the_package_does_not_import_its_heavy_submodule(package: str) -> None: + """The property the whole mechanism exists for, asserted directly. + + In a subprocess because this suite has already imported everything: a check + against the current ``sys.modules`` would pass or fail on test ordering rather + than on the package's behaviour. + """ + submodule = HEAVY_SUBMODULE[package] + code = ( + f"import {package}, sys; " + f"assert {submodule!r} not in sys.modules, {submodule!r} + ' was imported eagerly'" + ) + + result = subprocess.run( # noqa: S603 + [sys.executable, "-c", code], capture_output=True, text=True, check=False + ) + + assert result.returncode == 0, result.stderr