From 79567291b22ce148421518f0331e23c651570c03 Mon Sep 17 00:00:00 2001 From: Cody Fincher Date: Mon, 27 Jul 2026 17:27:12 +0000 Subject: [PATCH 1/2] feat: let any package ship extension migrations Extension migrations were discovered by mapping the extension_config key onto the hardcoded sqlspec.extensions. module namespace, so a package distributed separately from SQLSpec could not register migrations through any public surface. Doing so logged "Extension not found" and then silently registered nothing. Set migrations_path in an extension_config entry to point at a directory or a ':' specification. Declaring it auto-includes the extension, so it does not also need to appear in include_extensions; exclude_extensions still opts back out. Packages that register at runtime can call config.add_extension_migrations(name, path, settings) instead. Also fixes ext__ parsing for extension names containing underscores. A file such as ext_my_extension_0001_init.sql resolved to the version ext_my_extension, dropping 0001 and recording a malformed version in the tracking table. Three call sites now share parse_extension_stem, which anchors the version to the first all-numeric segment. Failures are now legible and proportionate: a malformed migrations_path raises MigrationError, a missing directory warns, an unimportable module warns naming the module and the consequence, and an extension that ships no migrations directory logs at debug rather than warning - six of the bundled extensions have none by design. module_to_os_path raises ModuleNotFoundError rather than TypeError so callers can catch the real condition, and resolves namespace packages to their search location instead of returning a path named "None". Claude-Session: https://claude.ai/code/session_01CKBtk2CVvc3nNRAx4NJ56K --- docs/changelog.rst | 33 +++ docs/usage/migrations.rst | 60 ++++++ sqlspec/config.py | 86 ++++++-- sqlspec/extensions/events/_store.py | 1 + sqlspec/extensions/litestar/store.py | 1 + sqlspec/migrations/base.py | 47 ++++- sqlspec/migrations/loaders.py | 12 +- sqlspec/migrations/runner.py | 19 +- sqlspec/migrations/utils.py | 99 ++++++++- sqlspec/migrations/version.py | 22 ++ sqlspec/utils/module_loader.py | 24 ++- .../test_upgrade_downgrade_versions.py | 64 ++++++ .../migrations/test_extension_discovery.py | 196 ++++++++++++++++++ .../test_extension_path_resolution.py | 89 ++++++++ .../unit/migrations/test_migration_runner.py | 46 ++++ tests/unit/migrations/test_version.py | 32 +++ tests/unit/utils/test_dependencies.py | 17 +- 17 files changed, 801 insertions(+), 47 deletions(-) create mode 100644 tests/unit/migrations/test_extension_path_resolution.py diff --git a/docs/changelog.rst b/docs/changelog.rst index e7f6379fa..322019689 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,39 @@ important operational fixes. Recent Updates ============== +v0.57.0 +------------------------------------------------------------------------------ + +**Added:** + +* Packages distributed separately from SQLSpec can now ship migrations. Set + ``migrations_path`` in an ``extension_config`` entry to point at a directory or + a ``':'`` specification, and the extension is discovered + and auto-included without appearing in ``include_extensions``. +* Added ``add_extension_migrations(name, migrations_path, settings=None)`` on + database configurations, for packages that register migrations at runtime + rather than declaratively. + +**Fixed:** + +* Extension names containing underscores no longer lose their version. A + migration such as ``ext_my_extension_0001_init.sql`` previously resolved to the + version ``ext_my_extension``, dropping ``0001`` and recording a malformed + version in the migration tracking table. +* A configured extension that cannot be resolved now reports which module was + tried and that no migrations were registered, instead of the ambiguous + ``Extension not found``. +* Extensions that ship no migrations directory no longer log a warning. Six of + the bundled extensions have no migrations by design, so the warning was noise. +* :func:`sqlspec.utils.module_loader.module_to_os_path` resolves namespace + packages to their search location instead of returning a path named ``None``. + +**Changed:** + +* :func:`sqlspec.utils.module_loader.module_to_os_path` raises + :class:`ModuleNotFoundError` rather than :class:`TypeError` when a module + cannot be found, so callers can catch the real condition. + v0.56.2 ------------------------------------------------------------------------------ diff --git a/docs/usage/migrations.rst b/docs/usage/migrations.rst index d4beaf87d..99dc22c99 100644 --- a/docs/usage/migrations.rst +++ b/docs/usage/migrations.rst @@ -15,6 +15,7 @@ Core Concepts - Migrations are SQL or Python files stored in a migrations directory. - Each database configuration can include its own migration settings. - Extension migrations (ADK, events, Litestar sessions) are opt-in and versioned. +- Any installed package can ship migrations, not just those under ``sqlspec.extensions``. Common Commands --------------- @@ -74,6 +75,65 @@ specific extension, ``migration_config["include_extensions"]`` to opt in explicitly by extension name, or ``migration_config["enabled"] = False`` to disable migrations entirely for a database config. +Third-Party Extension Migrations +-------------------------------- + +Extension names resolve against the ``sqlspec.extensions.`` namespace by +default. A package distributed separately from SQLSpec points at its own +migrations directory with ``migrations_path``: + +.. code-block:: python + + config = AsyncpgConfig( + connection_config={"dsn": "postgresql://localhost/app"}, + extension_config={ + "litestar_queues": { + "migrations_path": "litestar_queues.backends.sqlspec:migrations", + "table_name": "queue_tasks", + } + }, + ) + +Declaring ``migrations_path`` auto-includes the extension, so it does not also +need to appear in ``include_extensions``. ``exclude_extensions`` still opts it +back out. + +``migrations_path`` accepts either form: + +- A ``':'`` specification, resolved against the installed + package. Portable across machines, so this is the form to use in + ``[tool.sqlspec]`` configuration. +- A filesystem path, absolute or relative to the working directory, matching how + ``script_location`` resolves. + +Packages that register migrations at runtime can call +``add_extension_migrations`` instead of declaring the key: + +.. code-block:: python + + from pathlib import Path + + config.add_extension_migrations( + "litestar_queues", + Path(__file__).parent / "migrations", + settings={"table_name": "queue_tasks"}, + ) + +Both forms record the extension under ``extension_config`` and opt it into +``include_extensions``. Call ``add_extension_migrations`` before +``get_migration_commands()``; mutating ``extension_config`` directly after the +configuration is built does not re-run discovery. + +.. note:: + + Migrations are versioned under an ``ext_{name}_`` prefix, and that prefix is + written to the migration tracking table. Keep the extension name stable once + migrations have been applied — renaming it orphans the applied records. + + A package shipping migrations must include the directory as package data. If + it compiles its own modules, the migration sources must remain on disk, since + Python migrations are read and compiled at runtime. + Configuring a Default Schema ---------------------------- diff --git a/sqlspec/config.py b/sqlspec/config.py index 8432990d0..f726e55c9 100644 --- a/sqlspec/config.py +++ b/sqlspec/config.py @@ -11,7 +11,7 @@ import asyncio import threading from abc import ABC, abstractmethod -from collections.abc import Callable +from collections.abc import Callable, Mapping from inspect import Signature, signature from pathlib import Path from typing import TYPE_CHECKING, Any, ClassVar, Generic, Literal, TypeAlias, TypeVar, cast @@ -145,7 +145,8 @@ class MigrationConfig(TypedDict): """List of extension names whose migrations should be included. Extension migrations maintain separate versioning and are prefixed with 'ext_{name}_'. Note: Extensions with migration support (litestar, adk, events) are auto-included when - their settings are present in ``extension_config``. Use ``exclude_extensions`` to opt out. + their settings are present in ``extension_config``, as is any extension whose settings + declare ``migrations_path``. Use ``exclude_extensions`` to opt out. """ exclude_extensions: NotRequired["list[str]"] @@ -238,6 +239,13 @@ class LitestarConfig(TypedDict): All fields are optional with sensible defaults. """ + migrations_path: NotRequired[str | Path] + """Directory containing this extension's migrations, or a ``':'`` specification. + + Overrides the default ``sqlspec.extensions.`` lookup. Setting this auto-includes the + extension in ``migration_config["include_extensions"]``. + """ + session_table: NotRequired["bool | str"] """Enable session table for server-side session storage. @@ -476,6 +484,13 @@ class ADKConfig(TypedDict): 3. Selective features (sessions OR memory, not both) """ + migrations_path: NotRequired[str | Path] + """Directory containing this extension's migrations, or a ``':'`` specification. + + Overrides the default ``sqlspec.extensions.`` lookup. Setting this auto-includes the + extension in ``migration_config["include_extensions"]``. + """ + manage_schema: NotRequired[bool] """Apply additive target-schema reconciliation. Default: True.""" @@ -591,6 +606,13 @@ class EventsConfig(TypedDict): Use in ``extension_config["events"]``. """ + migrations_path: NotRequired[str | Path] + """Directory containing this extension's migrations, or a ``':'`` specification. + + Overrides the default ``sqlspec.extensions.`` lookup. Setting this auto-includes the + extension in ``migration_config["include_extensions"]``. + """ + manage_schema: NotRequired[bool] """Apply additive target-schema reconciliation. Default: True.""" @@ -944,6 +966,40 @@ def get_migration_commands(self) -> "SyncMigrationCommands[Any] | AsyncMigration """ return self._ensure_migration_commands() + def add_extension_migrations( + self, name: str, migrations_path: "str | Path", settings: "dict[str, Any] | None" = None + ) -> None: + """Register migrations shipped by a package outside the ``sqlspec.extensions`` namespace. + + Records the extension under ``extension_config``, opts it into + ``migration_config["include_extensions"]``, and rebuilds the cached migration + commands so the extension is discovered. Migrations are versioned under the + ``ext_{name}_`` prefix, so ``name`` must stay stable once migrations are applied. + + Args: + name: Extension name, used as the ``ext_{name}_`` version prefix. + migrations_path: Directory containing the migrations, or a + ``':'`` specification. + settings: Extension settings passed to its migrations. Merged into any + settings already registered under ``name``. + """ + extension_config = cast("dict[str, Any]", self.extension_config) + existing = extension_config.get(name) + merged: dict[str, Any] = dict(existing) if isinstance(existing, Mapping) else {} + if settings: + merged.update(settings) + merged["migrations_path"] = migrations_path + extension_config[name] = merged + + migration_config = cast("dict[str, Any]", self.migration_config) + include_extensions = migration_config.get("include_extensions") + include_list = list(include_extensions) if include_extensions else [] + if name not in include_list: + include_list.append(name) + migration_config["include_extensions"] = include_list + + self._rebuild_migration_commands() + @abstractmethod def migrate_up( self, @@ -1074,6 +1130,9 @@ def _ensure_extension_migrations(self) -> None: - **adk**: When any adk settings are present - **events**: When any events settings are present + Any other extension is auto-included when its settings declare ``migrations_path``, + which is how packages outside the ``sqlspec.extensions`` namespace ship migrations. + Use ``exclude_extensions`` to opt out of auto-inclusion. """ extension_settings = cast("dict[str, Any]", self.extension_config) @@ -1087,7 +1146,7 @@ def _ensure_extension_migrations(self) -> None: litestar_settings = extension_settings.get("litestar") if ( - litestar_settings is not None + isinstance(litestar_settings, Mapping) and "session_table" in litestar_settings and "litestar" not in exclude_extensions ): @@ -1104,22 +1163,21 @@ def _ensure_extension_migrations(self) -> None: if events_settings is not None and "events" not in exclude_extensions: extensions_to_add.append("events") + for ext_name, ext_settings in extension_settings.items(): + if ext_name in extensions_to_add or ext_name in exclude_extensions: + continue + if isinstance(ext_settings, Mapping) and "migrations_path" in ext_settings: + extensions_to_add.append(ext_name) + if not extensions_to_add: return include_extensions = migration_config.get("include_extensions") - if include_extensions is None: - include_list: list[str] = [] - migration_config["include_extensions"] = include_list - elif isinstance(include_extensions, tuple): - include_list = list(include_extensions) # pyright: ignore - migration_config["include_extensions"] = include_list - else: - include_list = cast("list[str]", include_extensions) - + include_list = list(include_extensions) if include_extensions else [] for ext in extensions_to_add: if ext not in include_list: include_list.append(ext) + migration_config["include_extensions"] = include_list def _build_storage_capabilities(self) -> "StorageCapabilities": arrow_dependency_needed = self.supports_native_arrow_export or self.supports_native_arrow_import @@ -1250,6 +1308,10 @@ def _initialize_migration_components(self) -> None: """Initialize migration loader and migration command helpers.""" runtime = self.get_observability_runtime() self._migration_loader = SQLFileLoader(runtime=runtime) + self._rebuild_migration_commands() + + def _rebuild_migration_commands(self) -> None: + """Rebuild the cached migration commands against current configuration.""" self._migration_commands = create_migration_commands(self) # pyright: ignore def _ensure_migration_loader(self) -> "SQLFileLoader": diff --git a/sqlspec/extensions/events/_store.py b/sqlspec/extensions/events/_store.py index d58d1f20e..55c73b694 100644 --- a/sqlspec/extensions/events/_store.py +++ b/sqlspec/extensions/events/_store.py @@ -40,6 +40,7 @@ class BaseEventQueueStore(ABC, Generic[ConfigT]): "event_poll_interval", "lease_seconds", "manage_schema", + "migrations_path", "poll_interval", "queue_table", "retention_seconds", diff --git a/sqlspec/extensions/litestar/store.py b/sqlspec/extensions/litestar/store.py index 9ca2017a9..4a1e0bb42 100644 --- a/sqlspec/extensions/litestar/store.py +++ b/sqlspec/extensions/litestar/store.py @@ -63,6 +63,7 @@ class BaseSQLSpecStore(ABC, Generic[ConfigT]): "extra_commit_statuses", "extra_rollback_statuses", "manage_schema", + "migrations_path", "pool_key", "run_migrations", "session_key", diff --git a/sqlspec/migrations/base.py b/sqlspec/migrations/base.py index d93a3355b..5ec5cff76 100644 --- a/sqlspec/migrations/base.py +++ b/sqlspec/migrations/base.py @@ -13,6 +13,7 @@ from sqlspec.exceptions import MigrationError from sqlspec.migrations.templates import MigrationTemplateSettings, build_template_settings from sqlspec.migrations.utils import resolve_default_schema as _resolve_default_schema +from sqlspec.migrations.utils import resolve_extension_migrations_path from sqlspec.migrations.utils import resolve_tracker_schema as _resolve_tracker_schema from sqlspec.migrations.version import parse_version from sqlspec.utils.logging import get_logger @@ -553,26 +554,52 @@ def _parse_extension_configs(self) -> "dict[str, dict[str, Any]]": def _discover_extension_migrations(self) -> "dict[str, Path]": """Discover migration paths for configured extensions. + An extension may declare ``migrations_path`` in its ``extension_config`` entry to + ship migrations from any package. Extensions without it resolve against the + ``sqlspec.extensions.`` namespace. + Returns: Dictionary mapping extension names to their migration paths. """ extension_migrations = {} - for ext_name in self.extension_configs: - module_name = "sqlspec.extensions.litestar" if ext_name == "litestar" else f"sqlspec.extensions.{ext_name}" + for ext_name, ext_options in self.extension_configs.items(): + spec = ext_options.get("migrations_path") + if spec is not None: + resolved = resolve_extension_migrations_path(ext_name, spec) + if resolved is None: + logger.warning( + "Extension '%s' migrations_path %r is not an existing directory; " + "no migrations were registered for it.", + ext_name, + spec, + ) + else: + extension_migrations[ext_name] = resolved + logger.debug("Found migrations for extension %s at %s", ext_name, resolved) + continue + module_name = f"sqlspec.extensions.{ext_name}" try: module_path = module_to_os_path(module_name) - migrations_dir = module_path / "migrations" + except ImportError: + logger.warning( + "Extension '%s' is configured but '%s' is not importable; no migrations were " + "registered for it. Set extension_config['%s']['migrations_path'] to register " + "migrations shipped by another package.", + ext_name, + module_name, + ext_name, + ) + continue - if migrations_dir.exists(): - extension_migrations[ext_name] = migrations_dir - logger.debug("Found migrations for extension %s at %s", ext_name, migrations_dir) - else: - logger.warning("No migrations directory found for extension %s", ext_name) - except TypeError: - logger.warning("Extension %s not found", ext_name) + migrations_dir = module_path / "migrations" + if migrations_dir.exists(): + extension_migrations[ext_name] = migrations_dir + logger.debug("Found migrations for extension %s at %s", ext_name, migrations_dir) + else: + logger.debug("Extension %s ships no migrations directory", ext_name) return extension_migrations diff --git a/sqlspec/migrations/loaders.py b/sqlspec/migrations/loaders.py index 8260fbbdb..fb1a2f326 100644 --- a/sqlspec/migrations/loaders.py +++ b/sqlspec/migrations/loaders.py @@ -14,7 +14,7 @@ from sqlspec.loader import SQLFileLoader as CoreSQLFileLoader from sqlspec.migrations.context import MigrationContext -from sqlspec.migrations.version import _format_sequential_version +from sqlspec.migrations.version import _format_sequential_version, parse_extension_stem from sqlspec.utils.sync_tools import await_ __all__ = ("BaseMigrationLoader", "MigrationLoadError", "PythonFileLoader", "SQLFileLoader", "get_migration_loader") @@ -169,16 +169,16 @@ def _extract_version(self, filename: str) -> str: Returns: Version string or empty string if invalid. """ - extension_version_parts = 3 timestamp_min_length = 4 name_without_ext = filename.rsplit(".", 1)[0] if name_without_ext.startswith("ext_"): - parts = name_without_ext.split("_", 3) - if len(parts) >= extension_version_parts: - return f"{parts[0]}_{parts[1]}_{parts[2]}" - return "" + extension = parse_extension_stem(name_without_ext) + if extension is None: + return "" + ext_name, ext_version = extension + return f"ext_{ext_name}_{ext_version}" parts = name_without_ext.split("_", 1) if parts and parts[0].isdigit(): diff --git a/sqlspec/migrations/runner.py b/sqlspec/migrations/runner.py index 307cd15cb..2e84388c1 100644 --- a/sqlspec/migrations/runner.py +++ b/sqlspec/migrations/runner.py @@ -15,7 +15,7 @@ from sqlspec.migrations.loaders import _load_migration_sql, get_migration_loader from sqlspec.migrations.templates import TemplateDescriptionHints from sqlspec.migrations.utils import resolve_default_schema as _resolve_default_schema -from sqlspec.migrations.version import _format_sequential_version, parse_version +from sqlspec.migrations.version import _format_sequential_version, parse_extension_stem, parse_version from sqlspec.observability import resolve_db_system from sqlspec.utils.logging import get_logger, log_with_context from sqlspec.utils.sync_tools import async_ @@ -238,16 +238,16 @@ def _extract_version(self, filename: str) -> "str | None": Returns: The extracted version string or None. """ - extension_version_parts = 3 timestamp_min_length = 4 name_without_ext = filename.rsplit(".", 1)[0] if name_without_ext.startswith("ext_"): - parts = name_without_ext.split("_", 3) - if len(parts) >= extension_version_parts: - return f"{parts[0]}_{parts[1]}_{parts[2]}" - return None + extension = parse_extension_stem(name_without_ext) + if extension is None: + return None + ext_name, ext_version = extension + return f"ext_{ext_name}_{ext_version}" parts = name_without_ext.split("_", 1) if parts and parts[0].isdigit(): @@ -435,10 +435,9 @@ def _migration_context(self, file_path: Path) -> "MigrationContext | None": if context_to_use and file_path.name.startswith("ext_"): version = self._extract_version(file_path.name) if version and version.startswith("ext_"): - min_extension_version_parts = 3 - parts = version.split("_", 2) - if len(parts) >= min_extension_version_parts: - ext_name = parts[1] + extension = parse_extension_stem(version) + if extension is not None: + ext_name = extension[0] if ext_name in self.extension_configs: context_to_use = MigrationContext( dialect=self.context.dialect if self.context else None, diff --git a/sqlspec/migrations/utils.py b/sqlspec/migrations/utils.py index 24540c5ff..d37a883a0 100644 --- a/sqlspec/migrations/utils.py +++ b/sqlspec/migrations/utils.py @@ -8,18 +8,28 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, cast +from sqlspec.exceptions import MigrationError from sqlspec.migrations.templates import MigrationTemplateSettings, TemplateValidationError, build_template_settings from sqlspec.utils.logging import get_logger +from sqlspec.utils.module_loader import module_to_os_path from sqlspec.utils.text import slugify if TYPE_CHECKING: from collections.abc import Callable, Mapping from sqlspec.config import DatabaseConfigProtocol -__all__ = ("create_migration_file", "get_author", "resolve_default_schema", "resolve_tracker_schema") +__all__ = ( + "create_migration_file", + "get_author", + "resolve_default_schema", + "resolve_extension_migrations_path", + "resolve_tracker_schema", +) logger = get_logger(__name__) +_MIN_MODULE_NAME_LENGTH = 2 + def resolve_default_schema(migration_config: "Mapping[str, Any] | None") -> str | None: """Resolve the configured default migration schema. @@ -149,6 +159,93 @@ def get_author( return _resolve_git_author() +def resolve_extension_migrations_path(ext_name: str, spec: "str | Path") -> "Path | None": + """Resolve an extension ``migrations_path`` setting to a migrations directory. + + Accepts a filesystem path, or a ``:`` specification resolved + against the installed package. Relative filesystem paths resolve against the current + working directory, matching ``script_location``. + + Args: + ext_name: Extension name the setting belongs to, used in error messages. + spec: Filesystem path or ``:`` specification. + + Raises: + MigrationError: The value is not a string or path, is empty, or names a module + that cannot be imported. + + Returns: + The resolved directory, or None when it does not exist. + """ + if isinstance(spec, Path): + candidate = spec + elif isinstance(spec, str): + if not spec.strip(): + msg = ( + f"Extension '{ext_name}' has an empty migrations_path; " + f"set extension_config['{ext_name}']['migrations_path'] to a directory " + f"or a ':' specification." + ) + raise MigrationError(msg) + candidate = _resolve_migrations_spec(ext_name, spec) + else: + msg = ( + f"Extension '{ext_name}' has an invalid migrations_path of type " + f"{type(spec).__name__}; set extension_config['{ext_name}']['migrations_path'] " + f"to a string or Path." + ) + raise MigrationError(msg) + + return candidate if candidate.is_dir() else None + + +def _resolve_migrations_spec(ext_name: str, spec: str) -> "Path": + """Resolve a string migrations_path to a directory, honoring module specifications. + + Args: + ext_name: Extension name the setting belongs to, used in error messages. + spec: Filesystem path or ``:`` specification. + + Raises: + MigrationError: The specification names a module that cannot be imported. + + Returns: + The resolved directory, which may not exist. + """ + module_name, separator, subdir = spec.partition(":") + if not separator or not _is_module_name(module_name): + return Path(spec) + + try: + module_path = module_to_os_path(module_name) + except ImportError as exc: + msg = ( + f"Extension '{ext_name}' migrations_path names module '{module_name}', " + f"which could not be imported ({exc}); correct " + f"extension_config['{ext_name}']['migrations_path'] or install the package." + ) + raise MigrationError(msg) from exc + + return module_path / subdir if subdir else module_path + + +def _is_module_name(value: str) -> bool: + r"""Return whether a string is a dotted Python module name. + + Single-character values are rejected so Windows drive letters in paths such as + ``C:\\srv\\migrations`` are treated as filesystem paths rather than modules. + + Args: + value: Candidate module name. + + Returns: + True when every dot-separated segment is a valid identifier. + """ + if len(value) < _MIN_MODULE_NAME_LENGTH: + return False + return all(segment.isidentifier() for segment in value.split(".")) + + def _get_git_config(config_key: str) -> str | None: """Retrieve git configuration value. diff --git a/sqlspec/migrations/version.py b/sqlspec/migrations/version.py index 851be35cc..1ea94a0fe 100644 --- a/sqlspec/migrations/version.py +++ b/sqlspec/migrations/version.py @@ -24,6 +24,7 @@ "get_next_sequential_number", "is_sequential_version", "is_timestamp_version", + "parse_extension_stem", "parse_version", ) @@ -32,6 +33,7 @@ SEQUENTIAL_PATTERN = re.compile(r"^(?!\d{14}$)\d+$") TIMESTAMP_PATTERN = re.compile(r"^(\d{14})$") EXTENSION_PATTERN = re.compile(r"^ext_(\w+)_(.+)$") +EXTENSION_STEM_PATTERN = re.compile(r"^ext_(?P\w+?)_(?P\d+)(?:_.*)?$") class VersionType(Enum): @@ -253,6 +255,26 @@ def parse_version(version_str: "str | None") -> MigrationVersion: raise ValueError(msg) +def parse_extension_stem(stem: str) -> "tuple[str, str] | None": + """Split an extension migration filename stem into its extension name and version. + + The version is the first all-numeric segment after the ``ext_`` prefix, so extension + names containing underscores resolve correctly: + ``ext_litestar_queues_0001_init`` yields ``("litestar_queues", "0001")``. + + Args: + stem: Filename stem without the file suffix, or an ``ext_``-prefixed version string. + + Returns: + Tuple of extension name and version, or None when the stem is not an + extension-prefixed migration. + """ + match = EXTENSION_STEM_PATTERN.match(stem) + if match is None: + return None + return match.group("name"), match.group("version") + + def _try_parse_version(version_str: str) -> "MigrationVersion | None": """Parse version string, returning None for invalid versions.""" try: diff --git a/sqlspec/utils/module_loader.py b/sqlspec/utils/module_loader.py index 5eee8238a..dbcbfcabe 100644 --- a/sqlspec/utils/module_loader.py +++ b/sqlspec/utils/module_loader.py @@ -240,24 +240,34 @@ def _resolve_import_attr(obj: Any, attr: str, module: "ModuleType | None", dotte def module_to_os_path(dotted_path: str = "app") -> "Path": """Convert a module dotted path to filesystem path. + Namespace packages have no ``origin``; their first search location is used instead. + Args: dotted_path: The path to the module. Raises: - TypeError: The module could not be found. + ModuleNotFoundError: The module could not be found. Returns: The path to the module. """ + msg = f"Couldn't find the path for {dotted_path}" try: - if (src := find_spec(dotted_path)) is None: # pragma: no cover - msg = f"Couldn't find the path for {dotted_path}" - raise TypeError(msg) + src = find_spec(dotted_path) except ModuleNotFoundError as e: - msg = f"Couldn't find the path for {dotted_path}" - raise TypeError(msg) from e + raise ModuleNotFoundError(msg, name=dotted_path) from e + + if src is None: + raise ModuleNotFoundError(msg, name=dotted_path) + + if not src.origin: + locations = iter(cast("Any", src).submodule_search_locations or ()) + first_location = next(locations, None) + if first_location is None: + raise ModuleNotFoundError(msg, name=dotted_path) + return Path(str(first_location)) - path = Path(str(src.origin)) + path = Path(src.origin) return path.parent if path.is_file() else path diff --git a/tests/integration/migrations/test_upgrade_downgrade_versions.py b/tests/integration/migrations/test_upgrade_downgrade_versions.py index ab45bbbc7..b3a679819 100644 --- a/tests/integration/migrations/test_upgrade_downgrade_versions.py +++ b/tests/integration/migrations/test_upgrade_downgrade_versions.py @@ -409,3 +409,67 @@ def test_downgrade_dry_run_shows_pending_downgrades(sqlite_config: SqliteConfig, assert "users" in tables assert "products" in tables + + +def test_upgrade_applies_third_party_extension_migrations(tmp_path: Path) -> None: + """A package outside sqlspec.extensions applies migrations through the public API. + + Covers both extension migration layouts: a file inside the extension's own directory, + and an ``ext_``-prefixed file sitting in the main migrations directory. + """ + migrations_dir = tmp_path / "migrations" + migrations_dir.mkdir() + vendor_dir = tmp_path / "vendor" / "migrations" + vendor_dir.mkdir(parents=True) + + (migrations_dir / "0001_core_init.sql").write_text("""-- name: migrate-0001-up +CREATE TABLE core (id INTEGER PRIMARY KEY); + +-- name: migrate-0001-down +DROP TABLE core; +""") + (vendor_dir / "0001_create_queue_tasks.py").write_text('''"""Create the vendor queue table.""" + +__all__ = ("down", "up") + + +def up(context: "object | None" = None) -> "list[str]": + """Return the upgrade statements.""" + return ["CREATE TABLE queue_tasks (id INTEGER PRIMARY KEY)"] + + +def down(context: "object | None" = None) -> "list[str]": + """Return the downgrade statements.""" + return ["DROP TABLE queue_tasks"] +''') + (migrations_dir / "ext_litestar_queues_0002_add_index.sql").write_text( + """-- name: migrate-ext_litestar_queues_0002-up +CREATE INDEX queue_tasks_id_idx ON queue_tasks (id); + +-- name: migrate-ext_litestar_queues_0002-down +DROP INDEX queue_tasks_id_idx; +""" + ) + + config = SqliteConfig( + connection_config={"database": str(tmp_path / "app.db")}, + migration_config={"script_location": str(migrations_dir), "version_table_name": "ddl_migrations"}, + ) + config.add_extension_migrations("litestar_queues", vendor_dir, settings={"table_name": "queue_tasks"}) + + try: + commands = SyncMigrationCommands(config) + commands.upgrade() + + with config.provide_session() as session: + applied = commands.tracker.get_applied_migrations(session) + tables = session.execute("SELECT name FROM sqlite_master WHERE type = 'table'").get_data() + + versions = [row["version_num"] for row in applied] + assert versions == ["0001", "ext_litestar_queues_0001", "ext_litestar_queues_0002"] + assert "queue_tasks" in {row["name"] for row in tables} + + commands.downgrade(revision="base") + assert commands.current() is None + finally: + config.close_pool() diff --git a/tests/unit/migrations/test_extension_discovery.py b/tests/unit/migrations/test_extension_discovery.py index f136931c9..81b807fc4 100644 --- a/tests/unit/migrations/test_extension_discovery.py +++ b/tests/unit/migrations/test_extension_discovery.py @@ -1,8 +1,12 @@ """Test extension migration discovery functionality.""" +import logging from pathlib import Path +import pytest + from sqlspec.adapters.sqlite.config import SqliteConfig +from sqlspec.exceptions import MigrationError from sqlspec.migrations.commands import SyncMigrationCommands @@ -78,3 +82,195 @@ def test_migration_file_discovery_with_extensions(tmp_path: Path) -> None: versions = [version for version, _ in migration_files] assert "0002" in versions + + +@pytest.fixture +def third_party_migrations(tmp_path: Path) -> Path: + """Create a migrations directory outside the sqlspec.extensions namespace.""" + ext_dir = tmp_path / "vendor" / "migrations" + ext_dir.mkdir(parents=True) + (ext_dir / "ext_litestar_queues_0001_init.sql").write_text(""" +-- name: migrate-ext_litestar_queues_0001-up +CREATE TABLE queue_tasks (id INTEGER PRIMARY KEY); + +-- name: migrate-ext_litestar_queues_0001-down +DROP TABLE queue_tasks; +""") + return ext_dir + + +def test_migrations_path_registers_third_party_directory(tmp_path: Path, third_party_migrations: Path) -> None: + """A package outside sqlspec.extensions registers migrations via migrations_path.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"litestar_queues": {"migrations_path": str(third_party_migrations)}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + commands = SyncMigrationCommands(config) + + assert commands.runner.extension_migrations["litestar_queues"] == third_party_migrations + + +def test_migrations_path_auto_includes_without_include_extensions(tmp_path: Path, third_party_migrations: Path) -> None: + """Declaring migrations_path opts the extension in without touching include_extensions.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"litestar_queues": {"migrations_path": str(third_party_migrations)}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + assert config.migration_config.get("include_extensions") == ["litestar_queues"] + + +def test_migrations_path_honors_exclude_extensions(tmp_path: Path, third_party_migrations: Path) -> None: + """exclude_extensions opts a third-party extension back out of auto-inclusion.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"litestar_queues": {"migrations_path": str(third_party_migrations)}}, + migration_config={"script_location": str(tmp_path / "migrations"), "exclude_extensions": ["litestar_queues"]}, + ) + + assert SyncMigrationCommands(config).runner.extension_migrations == {} + + +def test_migrations_path_accepts_module_specification(tmp_path: Path) -> None: + """A ':' value resolves against the installed package.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"vendor_ext": {"migrations_path": "sqlspec.extensions.adk:migrations"}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + resolved = SyncMigrationCommands(config).runner.extension_migrations["vendor_ext"] + assert resolved.is_dir() + assert resolved.name == "migrations" + + +def test_migrations_path_overrides_namespace_convention(tmp_path: Path, third_party_migrations: Path) -> None: + """An explicit migrations_path wins over the sqlspec.extensions. lookup.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"litestar": {"session_table": "s", "migrations_path": str(third_party_migrations)}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + assert SyncMigrationCommands(config).runner.extension_migrations["litestar"] == third_party_migrations + + +def test_missing_migrations_path_warns_and_registers_nothing(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """A migrations_path pointing nowhere warns instead of silently doing nothing.""" + caplog.set_level(logging.DEBUG, logger="sqlspec.migrations.base") + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"vendor_ext": {"migrations_path": str(tmp_path / "absent")}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + commands = SyncMigrationCommands(config) + + assert commands.runner.extension_migrations == {} + warnings = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING] + assert any("vendor_ext" in message and "not an existing directory" in message for message in warnings) + + +def test_invalid_migrations_path_raises(tmp_path: Path) -> None: + """A migrations_path of the wrong type is a configuration error, not a warning.""" + with pytest.raises(MigrationError, match="invalid migrations_path of type int"): + SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"vendor_ext": {"migrations_path": 42}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + +def test_unresolvable_extension_warning_names_module_and_consequence( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The warning states which module was tried and that nothing was registered.""" + caplog.set_level(logging.DEBUG, logger="sqlspec.migrations.base") + config = SqliteConfig( + connection_config={"database": ":memory:"}, + migration_config={"script_location": str(tmp_path), "include_extensions": ["litestar_queues"]}, + ) + + commands = SyncMigrationCommands(config) + + assert commands.runner.extension_migrations == {} + warnings = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING] + assert any( + "sqlspec.extensions.litestar_queues" in message + and "no migrations were registered" in message + and "migrations_path" in message + for message in warnings + ) + + +def test_shipped_extension_without_migrations_does_not_warn(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """Extensions that legitimately ship no migrations are not a warning condition.""" + caplog.set_level(logging.DEBUG, logger="sqlspec.migrations.base") + config = SqliteConfig( + connection_config={"database": ":memory:"}, + migration_config={"script_location": str(tmp_path), "include_extensions": ["fastapi"]}, + ) + + commands = SyncMigrationCommands(config) + + assert commands.runner.extension_migrations == {} + assert [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING] == [] + + +def test_add_extension_migrations_registers_after_construction(tmp_path: Path, third_party_migrations: Path) -> None: + """The public API registers an extension without touching runner internals.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, migration_config={"script_location": str(tmp_path / "migrations")} + ) + assert config.get_migration_commands().runner.extension_migrations == {} + + config.add_extension_migrations("litestar_queues", third_party_migrations, settings={"table_name": "queue_tasks"}) + + commands = config.get_migration_commands() + assert commands.runner.extension_migrations["litestar_queues"] == third_party_migrations + assert commands.extension_configs["litestar_queues"]["table_name"] == "queue_tasks" + assert config.migration_config.get("include_extensions") == ["litestar_queues"] + + +def test_add_extension_migrations_merges_existing_settings(tmp_path: Path, third_party_migrations: Path) -> None: + """Settings already registered under the extension name are preserved.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"litestar_queues": {"table_name": "original", "poll": 5}}, + migration_config={"script_location": str(tmp_path / "migrations")}, + ) + + config.add_extension_migrations("litestar_queues", third_party_migrations, settings={"table_name": "updated"}) + + settings = config.get_migration_commands().extension_configs["litestar_queues"] + assert settings["table_name"] == "updated" + assert settings["poll"] == 5 + + +def test_add_extension_migrations_preserves_loaded_sql(tmp_path: Path, third_party_migrations: Path) -> None: + """Rebuilding commands must not discard SQL already loaded into the migration loader.""" + config = SqliteConfig( + connection_config={"database": ":memory:"}, migration_config={"script_location": str(tmp_path / "migrations")} + ) + loader = config._ensure_migration_loader() + loader.add_named_sql("probe", "SELECT 1") + + config.add_extension_migrations("litestar_queues", third_party_migrations) + + assert "probe" in config._ensure_migration_loader().list_queries() + + +def test_auto_inclusion_does_not_mutate_caller_include_list(tmp_path: Path, third_party_migrations: Path) -> None: + """A shared include_extensions list is not mutated when configs auto-include extensions.""" + shared = ["fastapi"] + + SqliteConfig( + connection_config={"database": ":memory:"}, + extension_config={"litestar_queues": {"migrations_path": str(third_party_migrations)}}, + migration_config={"script_location": str(tmp_path / "migrations"), "include_extensions": shared}, + ) + + assert shared == ["fastapi"] diff --git a/tests/unit/migrations/test_extension_path_resolution.py b/tests/unit/migrations/test_extension_path_resolution.py new file mode 100644 index 000000000..d2bec4cd7 --- /dev/null +++ b/tests/unit/migrations/test_extension_path_resolution.py @@ -0,0 +1,89 @@ +"""Tests for resolving extension ``migrations_path`` settings.""" + +from pathlib import Path + +import pytest + +from sqlspec.exceptions import MigrationError +from sqlspec.migrations.utils import resolve_extension_migrations_path + + +def test_resolves_path_object(tmp_path: Path) -> None: + """A Path pointing at an existing directory resolves to itself.""" + assert resolve_extension_migrations_path("vendor", tmp_path) == tmp_path + + +def test_resolves_string_path(tmp_path: Path) -> None: + """A string filesystem path resolves to the matching directory.""" + assert resolve_extension_migrations_path("vendor", str(tmp_path)) == tmp_path + + +def test_resolves_module_specification() -> None: + """A ':' value resolves against the installed package.""" + resolved = resolve_extension_migrations_path("vendor", "sqlspec.extensions.adk:migrations") + + assert resolved is not None + assert resolved.is_dir() + assert resolved.name == "migrations" + + +def test_resolves_module_specification_without_subdir() -> None: + """A module specification with an empty subdir resolves to the module directory.""" + resolved = resolve_extension_migrations_path("vendor", "sqlspec.extensions.adk:") + + assert resolved is not None + assert resolved.name == "adk" + + +def test_missing_directory_returns_none(tmp_path: Path) -> None: + """A resolvable value naming a directory that does not exist returns None.""" + assert resolve_extension_migrations_path("vendor", tmp_path / "absent") is None + + +def test_file_target_returns_none(tmp_path: Path) -> None: + """A value naming a file rather than a directory returns None.""" + target = tmp_path / "migrations" + target.write_text("not a directory") + + assert resolve_extension_migrations_path("vendor", target) is None + + +@pytest.mark.parametrize( + "spec", + [ + r"C:\srv\migrations", + r"\\?\C:\srv\migrations", + "/opt/my:app/migrations", + ], +) +def test_path_like_values_are_not_treated_as_modules(spec: str) -> None: + """Drive letters and colons inside filesystem paths do not trigger module resolution.""" + assert resolve_extension_migrations_path("vendor", spec) is None + + +def test_bare_dotted_value_is_a_filesystem_path(tmp_path: Path) -> None: + """Without a ':' separator a dotted value is a filesystem path, not a module.""" + target = tmp_path / "sqlspec.extensions.adk" + target.mkdir() + + assert resolve_extension_migrations_path("vendor", str(target)) == target + + +def test_unimportable_module_raises() -> None: + """A module specification naming a missing package is a configuration error.""" + with pytest.raises(MigrationError, match="could not be imported"): + resolve_extension_migrations_path("vendor", "definitely_not_installed:migrations") + + +@pytest.mark.parametrize("spec", [42, None, 1.5, ["migrations"]]) +def test_non_string_value_raises(spec: object) -> None: + """A migrations_path that is not a string or Path is a configuration error.""" + with pytest.raises(MigrationError, match="invalid migrations_path of type"): + resolve_extension_migrations_path("vendor", spec) # type: ignore[arg-type] + + +@pytest.mark.parametrize("spec", ["", " "]) +def test_empty_value_raises(spec: str) -> None: + """An empty migrations_path is a configuration error.""" + with pytest.raises(MigrationError, match="empty migrations_path"): + resolve_extension_migrations_path("vendor", spec) diff --git a/tests/unit/migrations/test_migration_runner.py b/tests/unit/migrations/test_migration_runner.py index 63e8e1fb8..377ab9b61 100644 --- a/tests/unit/migrations/test_migration_runner.py +++ b/tests/unit/migrations/test_migration_runner.py @@ -1212,3 +1212,49 @@ async def test_no_reload() -> None: assert call_counts["load_sql"] == 1, "get_down_sql should NOT call load_sql (should use cache)" asyncio.run(test_no_reload()) + + +@pytest.mark.parametrize( + ("filename", "expected"), + [ + ("ext_litestar_0001_init.sql", "ext_litestar_0001"), + ("ext_litestar_queues_0001_init.sql", "ext_litestar_queues_0001"), + ("ext_litestar_queues_0002_add_index.sql", "ext_litestar_queues_0002"), + ("ext_adk_20251011120000_seed.sql", "ext_adk_20251011120000"), + ("0001_core.sql", "0001"), + ("20251011120000_core.sql", "20251011120000"), + ], +) +def test_extract_version_handles_multi_underscore_extension_names(tmp_path: Path, filename: str, expected: str) -> None: + """Extension names containing underscores keep their numeric version.""" + runner = SyncMigrationRunner(tmp_path) + + assert runner._extract_version(filename) == expected + + +def test_extract_version_result_parses_for_multi_underscore_extension(tmp_path: Path) -> None: + """The extracted version parses rather than falling back to raw string sorting.""" + from sqlspec.migrations.version import parse_version + + runner = SyncMigrationRunner(tmp_path) + + version = runner._extract_version("ext_litestar_queues_0001_init.sql") + assert version is not None + parsed = parse_version(version) + assert parsed.extension == "litestar_queues" + assert parsed.sequence == 1 + + +def test_migration_context_resolves_multi_underscore_extension_settings(tmp_path: Path) -> None: + """Per-migration context carries the settings of an underscore-named extension.""" + ext_dir = tmp_path / "vendor" + ext_dir.mkdir() + settings = {"table_name": "queue_tasks"} + runner = SyncMigrationRunner( + tmp_path, {"litestar_queues": ext_dir}, MigrationContext(dialect="sqlite"), {"litestar_queues": settings} + ) + + context = runner._migration_context(ext_dir / "ext_litestar_queues_0001_init.sql") + + assert context is not None + assert context.extension_config == settings diff --git a/tests/unit/migrations/test_version.py b/tests/unit/migrations/test_version.py index a52ca1dc6..c95dbf19e 100644 --- a/tests/unit/migrations/test_version.py +++ b/tests/unit/migrations/test_version.py @@ -15,6 +15,7 @@ generate_timestamp_version, is_sequential_version, is_timestamp_version, + parse_extension_stem, parse_version, ) @@ -326,3 +327,34 @@ def test_version_rich_comparison_migration_version_final_decorator_preserves_dat assert getattr(MigrationVersion, "__final__", True) is True assert copy.deepcopy(version) == version assert pickle.loads(pickle.dumps(version)) == version + + +@pytest.mark.parametrize( + ("stem", "expected"), + [ + ("ext_litestar_0001_init", ("litestar", "0001")), + ("ext_litestar_queues_0001_init", ("litestar_queues", "0001")), + ("ext_litestar_queues_0002_add_index", ("litestar_queues", "0002")), + ("ext_adk_20251011120000_seed", ("adk", "20251011120000")), + ("ext_myext_0001", ("myext", "0001")), + ("ext_a_b_c_0001", ("a_b_c", "0001")), + ("ext_litestar_0001", ("litestar", "0001")), + ], +) +def test_parse_extension_stem_splits_name_and_version(stem: str, expected: "tuple[str, str]") -> None: + """Extension names containing underscores keep their full name and numeric version.""" + assert parse_extension_stem(stem) == expected + + +@pytest.mark.parametrize("stem", ["ext_bogus", "ext_", "0001_core", "litestar_0001", "ext_noversion_here"]) +def test_parse_extension_stem_rejects_non_extension_stems(stem: str) -> None: + """Stems without an ``ext__`` shape return None.""" + assert parse_extension_stem(stem) is None + + +def test_parse_extension_stem_round_trips_through_parse_version() -> None: + """A multi-underscore extension version parses instead of falling back to string sort.""" + ext_name, version = parse_extension_stem("ext_litestar_queues_0001_init") # type: ignore[misc] + parsed = parse_version(f"ext_{ext_name}_{version}") + assert parsed.extension == "litestar_queues" + assert parsed.sequence == 1 diff --git a/tests/unit/utils/test_dependencies.py b/tests/unit/utils/test_dependencies.py index dfd3d9e71..1ceefcc53 100644 --- a/tests/unit/utils/test_dependencies.py +++ b/tests/unit/utils/test_dependencies.py @@ -265,10 +265,25 @@ def test_module_to_os_path_current_package() -> None: def test_module_to_os_path_nonexistent() -> None: """Test module_to_os_path with nonexistent module.""" - with pytest.raises(TypeError, match="Couldn't find the path"): + with pytest.raises(ModuleNotFoundError, match="Couldn't find the path"): module_to_os_path("definitely.nonexistent.module") +def test_module_to_os_path_missing_submodule_of_real_package() -> None: + """Test module_to_os_path when the parent package exists but the submodule does not.""" + with pytest.raises(ModuleNotFoundError, match="Couldn't find the path"): + module_to_os_path("sqlspec.extensions.definitely_not_an_extension") + + +def test_module_to_os_path_namespace_package() -> None: + """Test module_to_os_path resolves namespace packages to a search location.""" + pytest.importorskip("google") + path = module_to_os_path("google") + assert isinstance(path, Path) + assert path.is_dir() + assert path.name == "google" + + def test_module_to_os_path_file_module() -> None: """Test module_to_os_path returns parent for file modules.""" path = module_to_os_path("sqlspec.exceptions") From 516c247d29907e27327b739611559c8f43576271 Mon Sep 17 00:00:00 2001 From: Cody Fincher Date: Mon, 27 Jul 2026 20:18:40 +0000 Subject: [PATCH 2/2] style: apply ruff formatting to extension path resolution tests Claude-Session: https://claude.ai/code/session_01CKBtk2CVvc3nNRAx4NJ56K --- tests/unit/migrations/test_extension_path_resolution.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/unit/migrations/test_extension_path_resolution.py b/tests/unit/migrations/test_extension_path_resolution.py index d2bec4cd7..3e61988ba 100644 --- a/tests/unit/migrations/test_extension_path_resolution.py +++ b/tests/unit/migrations/test_extension_path_resolution.py @@ -48,14 +48,7 @@ def test_file_target_returns_none(tmp_path: Path) -> None: assert resolve_extension_migrations_path("vendor", target) is None -@pytest.mark.parametrize( - "spec", - [ - r"C:\srv\migrations", - r"\\?\C:\srv\migrations", - "/opt/my:app/migrations", - ], -) +@pytest.mark.parametrize("spec", [r"C:\srv\migrations", r"\\?\C:\srv\migrations", "/opt/my:app/migrations"]) def test_path_like_values_are_not_treated_as_modules(spec: str) -> None: """Drive letters and colons inside filesystem paths do not trigger module resolution.""" assert resolve_extension_migrations_path("vendor", spec) is None