From 836f7f30ae602ae530611fcda5c2de7107de7c56 Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 02:28:25 -0400 Subject: [PATCH 1/4] Discover validation registry via entry points REGISTRY was built by walking expressions/generated/ with pathlib.Path.rglob, which only sees real filesystem directories. Loaded from a wheel on sys.path (zipimport) instead of extracted -- as AWS Glue does via --extra-py-files -- the generated modules live inside the zip, pathlib finds nothing, and every feature type reports as unregistered. Declare one overture.pyspark_validations entry point per generated module and read them through importlib.metadata, which resolves from dist metadata regardless of how the package is loaded. The table is hand-maintained like the theme packages' overture.models entry points; a new test asserts it matches the generated tree so a forgotten or stale declaration fails CI rather than silently dropping a feature. An entry point whose module is absent is skipped, preserving the empty-registry-on-missing-tree behavior for a build without the generated modules. Signed-off-by: Adam Lastowka --- .../changelog.d/661.bugfix.md | 1 + .../overture-schema-pyspark/pyproject.toml | 30 ++++++++ .../src/overture/schema/pyspark/_registry.py | 64 ++++++----------- .../tests/test_registry.py | 69 ++++++++++++++----- 4 files changed, 102 insertions(+), 62 deletions(-) create mode 100644 packages/overture-schema-pyspark/changelog.d/661.bugfix.md diff --git a/packages/overture-schema-pyspark/changelog.d/661.bugfix.md b/packages/overture-schema-pyspark/changelog.d/661.bugfix.md new file mode 100644 index 000000000..ea040cd5f --- /dev/null +++ b/packages/overture-schema-pyspark/changelog.d/661.bugfix.md @@ -0,0 +1 @@ +Fixed the validation registry coming back empty when `overture-schema-pyspark` is loaded from a wheel on `sys.path` (zipimport) rather than installed to a real directory, as happens on AWS Glue via `--extra-py-files`. The registry now discovers generated modules through `overture.pyspark_validations` entry points read via `importlib.metadata`, which resolve whether the package lives on disk or inside a zip archive. diff --git a/packages/overture-schema-pyspark/pyproject.toml b/packages/overture-schema-pyspark/pyproject.toml index 6e72bc93c..9445586ff 100644 --- a/packages/overture-schema-pyspark/pyproject.toml +++ b/packages/overture-schema-pyspark/pyproject.toml @@ -17,6 +17,36 @@ version = "0.1.1" [project.scripts] overture-validate = "overture.schema.pyspark.cli:validate_cli" +# One entry point per generated validation, keyed by feature type, pointing at +# the module's MODEL_VALIDATION. The registry reads these through +# importlib.metadata, which resolves the same whether the package is installed +# to a directory or loaded as a wheel on sys.path -- so discovery no longer +# depends on the generated tree being reachable as filesystem paths. +# +# This list is maintained by hand, the same way theme packages declare their +# `overture.models` entry points, and changes only when a feature type is +# added or removed. `tests/test_registry.py` fails if it drifts from the tree +# `make generate-pyspark` produces. Values point into that generated tree, +# which is built rather than committed; before it exists an entry point cannot +# load, and the registry skips it (an absent tree yields an empty registry, as +# documented). +[project.entry-points."overture.pyspark_validations"] +address = "overture.schema.pyspark.expressions.generated.overture.schema.addresses.address:MODEL_VALIDATION" +bathymetry = "overture.schema.pyspark.expressions.generated.overture.schema.base.bathymetry:MODEL_VALIDATION" +infrastructure = "overture.schema.pyspark.expressions.generated.overture.schema.base.infrastructure:MODEL_VALIDATION" +land = "overture.schema.pyspark.expressions.generated.overture.schema.base.land:MODEL_VALIDATION" +land_cover = "overture.schema.pyspark.expressions.generated.overture.schema.base.land_cover:MODEL_VALIDATION" +land_use = "overture.schema.pyspark.expressions.generated.overture.schema.base.land_use:MODEL_VALIDATION" +water = "overture.schema.pyspark.expressions.generated.overture.schema.base.water:MODEL_VALIDATION" +building = "overture.schema.pyspark.expressions.generated.overture.schema.buildings.building:MODEL_VALIDATION" +building_part = "overture.schema.pyspark.expressions.generated.overture.schema.buildings.building_part:MODEL_VALIDATION" +division = "overture.schema.pyspark.expressions.generated.overture.schema.divisions.division:MODEL_VALIDATION" +division_area = "overture.schema.pyspark.expressions.generated.overture.schema.divisions.division_area:MODEL_VALIDATION" +division_boundary = "overture.schema.pyspark.expressions.generated.overture.schema.divisions.division_boundary:MODEL_VALIDATION" +place = "overture.schema.pyspark.expressions.generated.overture.schema.places.place:MODEL_VALIDATION" +connector = "overture.schema.pyspark.expressions.generated.overture.schema.transportation.connector:MODEL_VALIDATION" +segment = "overture.schema.pyspark.expressions.generated.overture.schema.transportation.segment:MODEL_VALIDATION" + [tool.uv.build-backend] module-name = "overture.schema.pyspark" diff --git a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py index d5c20ad5d..b4e383d23 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -1,50 +1,32 @@ """Runtime registry of feature validations. -Built at import time by walking the generated `expressions.generated` -namespace and collecting every module that exposes the -codegen-emitted `ENTRY_POINT` and `MODEL_VALIDATION` constants. - -The generated tree on disk is the runtime source of truth: the -registry contains exactly what was generated, regardless of which -theme packages are installed alongside the pyspark package. A missing -`expressions/generated/` subtree simply yields an empty registry -- -the package still imports cleanly. +Built at import time from the `overture.pyspark_validations` entry points, +one per generated validation module, declared by this package. Reading them +through `importlib.metadata` resolves the same whether the package is +installed to a real directory or loaded straight from a wheel on `sys.path` +(as on Glue, via `--extra-py-files`), so discovery does not depend on the +generated tree being reachable as filesystem paths. + +An entry point whose module is absent -- a build without the generated tree, +or a stale declaration -- is skipped, so a package missing its generated +modules still imports cleanly with an empty registry. """ from __future__ import annotations import importlib +import importlib.metadata import logging -from pathlib import Path from .check import ModelValidation logger = logging.getLogger(__name__) -_GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" - - -def _iter_generated_module_names(root_paths: list[str]) -> list[str]: - """Return the dotted names of every generated module on disk. - - The generated tree is PEP 420 (no `__init__.py`), so its subdirectories - are namespace packages. `pkgutil.walk_packages` skips those, so the tree - is walked as files instead: every `.py` under the namespace roots, keyed - to a dotted name relative to `_GENERATED_ROOT`. - """ - names: list[str] = [] - for root_path in root_paths: - base = Path(root_path) - for path in sorted(base.rglob("*.py")): - if path.name == "__init__.py": - continue - relative = path.relative_to(base).with_suffix("") - names.append(".".join([_GENERATED_ROOT, *relative.parts])) - return names +_ENTRY_POINT_GROUP = "overture.pyspark_validations" def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: - """Walk the generated tree and collect registry + partition map. + """Collect registry + partition map from the validation entry points. Returns a `(registry, partition_map)` pair: @@ -54,20 +36,17 @@ def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: "place"}`) for path construction. Features with no `PARTITIONS` data (empty dict) are omitted; the codegen only sets `PARTITIONS` when the data lake organizes the feature by Hive partitions. - `type` is appended here from the module file name so consumers - get a complete partition path without the codegen having to - duplicate the type value. + `type` comes from the entry-point name so consumers get a complete + partition path without the codegen having to duplicate the value. """ registry: dict[str, ModelValidation] = {} partition_map: dict[str, dict[str, str]] = {} - try: - root = importlib.import_module(_GENERATED_ROOT) - except ImportError: - return registry, partition_map - - for name in _iter_generated_module_names(list(root.__path__)): - module = importlib.import_module(name) + for ep in importlib.metadata.entry_points(group=_ENTRY_POINT_GROUP): + try: + module = importlib.import_module(ep.module) + except ModuleNotFoundError: + continue entry_point = getattr(module, "ENTRY_POINT", None) validation = getattr(module, "MODEL_VALIDATION", None) if entry_point is None or validation is None: @@ -75,8 +54,7 @@ def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: registry[entry_point] = validation partitions = getattr(module, "PARTITIONS", None) or {} if partitions: - feature_type = name.rsplit(".", 1)[-1] - partition_map[entry_point] = {**partitions, "type": feature_type} + partition_map[entry_point] = {**partitions, "type": ep.name} return registry, partition_map diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index fcf261207..3d5e4c548 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -1,15 +1,17 @@ """Tests for the runtime registry's discovery of the generated tree. -The generated expression tree is PEP 420 (no `__init__.py`), so the -registry must walk it as a namespace package. No other test exercises the -real on-disk walk -- conformance tests import expression modules directly -and `test_validate.py` registers models through a test shim -- so an empty -registry would otherwise pass the suite unnoticed. +The registry is built from the `overture.pyspark_validations` entry points, +a hand-maintained table in `pyproject.toml`. These tests hold that table to +the generated tree `make generate-pyspark` produces: one asserts the +registry actually populates, the other that the declared entry points match +the modules on disk exactly, so a forgotten or stale declaration fails here +rather than silently dropping a feature type at runtime. """ from __future__ import annotations import importlib +import importlib.metadata from pathlib import Path import pytest @@ -17,32 +19,61 @@ from overture.schema.pyspark._registry import REGISTRY _GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" +_ENTRY_POINT_GROUP = "overture.pyspark_validations" -def _generated_leaf_count() -> int: - """Count generated model modules on disk (excludes namespace dirs). +def _generated_module_names() -> set[str]: + """Dotted names of every generated model module on disk. - Returns 0 when the generated tree is absent -- mirroring the registry's - own `ImportError` handling -- so the test skips rather than errors. + Empty when the generated tree is absent -- mirroring the registry's own + handling -- so the tests skip rather than error. """ try: root = importlib.import_module(_GENERATED_ROOT) except ImportError: - return 0 - return sum( - 1 - for base in root.__path__ - for path in Path(base).rglob("*.py") - if path.name != "__init__.py" - ) + return set() + names: set[str] = set() + for base in root.__path__: + for path in Path(base).rglob("*.py"): + if path.name == "__init__.py": + continue + relative = path.relative_to(base).with_suffix("") + names.add(".".join([_GENERATED_ROOT, *relative.parts])) + return names + + +def _declared_entry_point_modules() -> set[str]: + """Modules targeted by the declared `overture.pyspark_validations` entry points.""" + eps = importlib.metadata.entry_points(group=_ENTRY_POINT_GROUP) + return {ep.module for ep in eps} def test_registry_discovers_generated_models() -> None: - """The registry finds generated modules under the PEP 420 namespace tree.""" - if _generated_leaf_count() == 0: + """The registry finds generated modules through the declared entry points.""" + if not _generated_module_names(): pytest.skip("generated tree not present; run `make generate-pyspark`") generated_entries = [ key for key in REGISTRY if ":" in key and key.startswith("overture.schema.") ] - assert generated_entries, "registry found no generated feature modules on disk" + assert generated_entries, "registry found no generated feature modules" + + +def test_entry_points_match_generated_tree() -> None: + """Declared entry points match the generated modules on disk exactly. + + Catches a new feature whose entry point was never declared (present on + disk, missing from the table) and a stale declaration left behind after + a feature was removed (in the table, absent on disk). + """ + on_disk = _generated_module_names() + if not on_disk: + pytest.skip("generated tree not present; run `make generate-pyspark`") + + declared = _declared_entry_point_modules() + assert declared == on_disk, ( + "overture.pyspark_validations entry points are out of sync with the " + "generated tree.\n" + f" declared but not on disk: {sorted(declared - on_disk)}\n" + f" on disk but not declared: {sorted(on_disk - declared)}" + ) From c6b91167ccfc314cab759a6dffd5b8603a8a2e98 Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 02:50:34 -0400 Subject: [PATCH 2/4] Narrow registry import guard and filter to own dist Two review fixes on the entry-point discovery: Catching ModuleNotFoundError unconditionally would also swallow a real missing dependency inside a present generated module, silently dropping that validation -- the same empty-registry failure this change exists to prevent. Skip only when the generated module (or an ancestor namespace) is absent; re-raise when a present module fails to import. importlib.metadata.entry_points(group=...) returns entries from every installed distribution, so a foreign package declaring the same group could inject validations or skew the drift test. Filter to this distribution's own entry points, shared by the registry and the test. Signed-off-by: Adam Lastowka --- .../src/overture/schema/pyspark/_registry.py | 55 ++++++++++++++----- .../tests/test_registry.py | 14 +++-- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py index b4e383d23..ecc4b7bdc 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -1,15 +1,17 @@ """Runtime registry of feature validations. -Built at import time from the `overture.pyspark_validations` entry points, -one per generated validation module, declared by this package. Reading them -through `importlib.metadata` resolves the same whether the package is -installed to a real directory or loaded straight from a wheel on `sys.path` -(as on Glue, via `--extra-py-files`), so discovery does not depend on the -generated tree being reachable as filesystem paths. - -An entry point whose module is absent -- a build without the generated tree, -or a stale declaration -- is skipped, so a package missing its generated -modules still imports cleanly with an empty registry. +Built at import time from this package's `overture.pyspark_validations` +entry points, one per generated validation module. Reading them through +`importlib.metadata` resolves the same whether the package is installed to a +real directory or loaded straight from a wheel on `sys.path` (as on Glue, via +`--extra-py-files`), so discovery does not depend on the generated tree being +reachable as filesystem paths. + +An entry point whose module is simply absent -- a build without the generated +tree, or a stale declaration -- is skipped, so a package missing its generated +modules still imports cleanly with an empty registry. A module that is present +but fails to import (a missing dependency, a codegen bug) raises, rather than +silently dropping that validation. """ from __future__ import annotations @@ -23,6 +25,26 @@ logger = logging.getLogger(__name__) _ENTRY_POINT_GROUP = "overture.pyspark_validations" +_DIST_NAME = "overture-schema-pyspark" + + +def _canonical(name: str) -> str: + """Normalize a distribution name for comparison (PEP 503).""" + return name.replace("_", "-").lower() + + +def _own_entry_points() -> list[importlib.metadata.EntryPoint]: + """This distribution's validation entry points. + + `importlib.metadata.entry_points(group=...)` returns matching entry points + from every installed distribution. Filtering to this one keeps a foreign + package that happens to declare the same group from injecting validations. + """ + return [ + ep + for ep in importlib.metadata.entry_points(group=_ENTRY_POINT_GROUP) + if _canonical(getattr(ep.dist, "name", "") or "") == _DIST_NAME + ] def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: @@ -42,11 +64,18 @@ def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: registry: dict[str, ModelValidation] = {} partition_map: dict[str, dict[str, str]] = {} - for ep in importlib.metadata.entry_points(group=_ENTRY_POINT_GROUP): + for ep in _own_entry_points(): try: module = importlib.import_module(ep.module) - except ModuleNotFoundError: - continue + except ModuleNotFoundError as e: + missing = e.name or "" + # Skip only when the generated module itself (or an ancestor + # namespace of it) is absent -- a build without the generated tree. + # A dependency missing *inside* a module that is present is a real + # failure; re-raise it instead of silently dropping the validation. + if missing == ep.module or ep.module.startswith(f"{missing}."): + continue + raise entry_point = getattr(module, "ENTRY_POINT", None) validation = getattr(module, "MODEL_VALIDATION", None) if entry_point is None or validation is None: diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index 3d5e4c548..04b254ce1 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -11,15 +11,13 @@ from __future__ import annotations import importlib -import importlib.metadata from pathlib import Path import pytest -from overture.schema.pyspark._registry import REGISTRY +from overture.schema.pyspark._registry import REGISTRY, _own_entry_points _GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" -_ENTRY_POINT_GROUP = "overture.pyspark_validations" def _generated_module_names() -> set[str]: @@ -43,9 +41,13 @@ def _generated_module_names() -> set[str]: def _declared_entry_point_modules() -> set[str]: - """Modules targeted by the declared `overture.pyspark_validations` entry points.""" - eps = importlib.metadata.entry_points(group=_ENTRY_POINT_GROUP) - return {ep.module for ep in eps} + """Modules targeted by this distribution's declared entry points. + + Uses the registry's own filtered lookup so the test checks exactly the + table the registry reads, without picking up a same-named group from + another distribution in the environment. + """ + return {ep.module for ep in _own_entry_points()} def test_registry_discovers_generated_models() -> None: From 605c308bffb0875727eaace136ec31275b544126 Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 02:55:14 -0400 Subject: [PATCH 3/4] Reword docstring to imperative mood docformat (D401/D404) rejects a docstring first line that opens with "This"; lead with the verb instead. Signed-off-by: Adam Lastowka --- .../src/overture/schema/pyspark/_registry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py index ecc4b7bdc..6f2d4552d 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -34,7 +34,7 @@ def _canonical(name: str) -> str: def _own_entry_points() -> list[importlib.metadata.EntryPoint]: - """This distribution's validation entry points. + """Return this distribution's own validation entry points. `importlib.metadata.entry_points(group=...)` returns matching entry points from every installed distribution. Filtering to this one keeps a foreign From ad77f79d6dba9e034972c5ab8da0a1b4da93fe0a Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 03:13:27 -0400 Subject: [PATCH 4/4] Discover validation registry via a generated index module Replace the entry-point table approach with a codegen-emitted `_index` module. `make generate-pyspark` now writes an `_index.py` alongside the validation modules that imports each one and exposes them as `MODULES`; the runtime registry imports that index instead of walking the generated tree on disk. An ordinary import resolves whether the package is installed to a directory or loaded as a wheel on sys.path (zipimport), which is what AWS Glue does via --extra-py-files and where the on-disk walk found nothing. This drops the hand-maintained entry-point table entirely: the index is generated from the same specs as the modules, so it cannot drift, and the registry-building policy stays in _registry.py rather than in package metadata. A test asserts the index covers exactly the modules on disk, guarding the codegen step. The index is a plain `_index.py`, not an `__init__.py`, so the generated tree stays PEP 420. A build without the generated tree has no index and yields an empty registry; a present-but-broken module raises instead of being silently skipped. Signed-off-by: Adam Lastowka --- .../changelog.d/661.misc.md | 1 + .../schema/codegen/pyspark/pipeline.py | 50 +++++++++++- .../tests/test_pyspark_pipeline.py | 16 +++- .../changelog.d/661.bugfix.md | 2 +- .../overture-schema-pyspark/pyproject.toml | 30 -------- .../src/overture/schema/pyspark/_registry.py | 77 +++++++------------ .../tests/test_registry.py | 54 ++++++------- 7 files changed, 112 insertions(+), 118 deletions(-) create mode 100644 packages/overture-schema-codegen/changelog.d/661.misc.md diff --git a/packages/overture-schema-codegen/changelog.d/661.misc.md b/packages/overture-schema-codegen/changelog.d/661.misc.md new file mode 100644 index 000000000..f373987a8 --- /dev/null +++ b/packages/overture-schema-codegen/changelog.d/661.misc.md @@ -0,0 +1 @@ +The PySpark generator now emits an `_index` module listing the generated validation modules, which the runtime registry imports to discover them without walking the generated tree on disk. diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/pipeline.py b/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/pipeline.py index 3b83f5448..65fbaea2e 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/pipeline.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/pipeline.py @@ -60,6 +60,16 @@ class PipelineOutput: _OUTPUT_PACKAGE = "overture.schema.pyspark.expressions.generated" +_INDEX_PATH = PurePosixPath("_index.py") + +_INDEX_DOCSTRING = '''"""Index of the generated validation modules. + +The runtime registry imports this module and reads ``MODULES``. Because that is +an ordinary import, discovery works even when the package is loaded from a wheel +on ``sys.path`` (zipimport), as AWS Glue does via ``--extra-py-files``, where the +generated modules are reachable by import but not as files on disk. +"""''' + def _require_entry_point(spec: ModelSpec) -> str: """Return *spec*'s entry point or raise if it's missing.""" @@ -130,17 +140,53 @@ def generate_pyspark_modules( Returns ------- PipelineOutput - Source-tree model modules and test-tree modules. The generated - tree is PEP 420, so no `__init__.py` files are emitted. + Source-tree model modules plus an `_index` module listing them, + and test-tree modules. The index is an ordinary `_index.py`, so the + generated tree stays PEP 420 and its namespace packages are untouched. """ items = [(spec, build_checks(spec)) for spec in model_specs] source = [_render_module(spec, checks) for spec, checks in items] + if source: + source.append(_render_index(model_specs)) test: list[GeneratedModule] = [] for spec, checks in items: test.extend(_render_test_modules(spec, checks)) return PipelineOutput(source=source, test=test) +def _render_index(model_specs: Sequence[ModelSpec]) -> GeneratedModule: + """Render the `_index` module listing every generated validation module. + + The runtime registry imports the generated modules through this index, so + the codegen owns which modules exist. Each is aliased by its full dotted + path so two feature types with the same leaf name cannot collide. + """ + entries = sorted( + ( + ".".join([_OUTPUT_PACKAGE, *directory.parts]), + model_name, + "_".join([*directory.parts, model_name]), + ) + for directory, model_name in ( + _directory_and_model_name(spec) for spec in model_specs + ) + ) + lines = [ + "# This file is auto-generated by overture-schema-codegen. Do not edit.", + _INDEX_DOCSTRING, + "", + "from __future__ import annotations", + "", + *(f"from {parent} import {leaf} as {alias}" for parent, leaf, alias in entries), + "", + "MODULES = (", + *(f" {alias}," for _, _, alias in entries), + ")", + "", + ] + return GeneratedModule(content="\n".join(lines), path=_INDEX_PATH) + + def _render_module( spec: ModelSpec, checks: tuple[list[Check], list[ModelCheck]], diff --git a/packages/overture-schema-codegen/tests/test_pyspark_pipeline.py b/packages/overture-schema-codegen/tests/test_pyspark_pipeline.py index 1c04c94a4..772afb7db 100644 --- a/packages/overture-schema-codegen/tests/test_pyspark_pipeline.py +++ b/packages/overture-schema-codegen/tests/test_pyspark_pipeline.py @@ -94,7 +94,15 @@ def test_empty_specs_returns_no_modules(self) -> None: assert result.test == [] def test_one_module_per_spec(self, two_spec_modules: PipelineOutput) -> None: - assert len(two_spec_modules.source) == 2 + model_modules = [ + m for m in two_spec_modules.source if m.path.name != "_index.py" + ] + assert len(model_modules) == 2 + + def test_emits_index_module(self, two_spec_modules: PipelineOutput) -> None: + index = [m for m in two_spec_modules.source if m.path.name == "_index.py"] + assert len(index) == 1 + assert index[0].path == PurePosixPath("_index.py") def test_paths_unique_per_tree(self, two_spec_modules: PipelineOutput) -> None: # source and test trees mirror the same dirs; uniqueness is @@ -236,7 +244,7 @@ def test_module_path_mirrors_entry_point(self) -> None: SimpleModel, entry_point="overture.schema.simple:SimpleModel" ) modules = generate_pyspark_modules([spec]) - features = modules.source + features = [m for m in modules.source if m.path.name != "_index.py"] assert len(features) == 1 assert features[0].path == PurePosixPath( "overture/schema/simple/simple_model.py" @@ -264,8 +272,8 @@ def test_neither_tree_has_init_modules(self) -> None: class TestNoRegistryEmitted: def test_registry_module_is_no_longer_generated(self) -> None: - # The runtime builds the registry via entry-point discovery; codegen - # must not emit `_registry.py`. + # The runtime builds the registry from the generated `_index` module; + # codegen emits that index but never a hand-written `_registry.py`. spec = extract_model( SimpleModel, entry_point="overture.schema.simple:SimpleModel" ) diff --git a/packages/overture-schema-pyspark/changelog.d/661.bugfix.md b/packages/overture-schema-pyspark/changelog.d/661.bugfix.md index ea040cd5f..07a043ed6 100644 --- a/packages/overture-schema-pyspark/changelog.d/661.bugfix.md +++ b/packages/overture-schema-pyspark/changelog.d/661.bugfix.md @@ -1 +1 @@ -Fixed the validation registry coming back empty when `overture-schema-pyspark` is loaded from a wheel on `sys.path` (zipimport) rather than installed to a real directory, as happens on AWS Glue via `--extra-py-files`. The registry now discovers generated modules through `overture.pyspark_validations` entry points read via `importlib.metadata`, which resolve whether the package lives on disk or inside a zip archive. +Fixed the validation registry coming back empty when `overture-schema-pyspark` is loaded from a wheel on `sys.path` (zipimport) rather than installed to a real directory, as happens on AWS Glue via `--extra-py-files`. Codegen now emits an `_index` module listing the generated validation modules, and the registry imports it instead of walking the generated tree, so discovery works whether the package lives on disk or inside a zip archive. diff --git a/packages/overture-schema-pyspark/pyproject.toml b/packages/overture-schema-pyspark/pyproject.toml index 9445586ff..6e72bc93c 100644 --- a/packages/overture-schema-pyspark/pyproject.toml +++ b/packages/overture-schema-pyspark/pyproject.toml @@ -17,36 +17,6 @@ version = "0.1.1" [project.scripts] overture-validate = "overture.schema.pyspark.cli:validate_cli" -# One entry point per generated validation, keyed by feature type, pointing at -# the module's MODEL_VALIDATION. The registry reads these through -# importlib.metadata, which resolves the same whether the package is installed -# to a directory or loaded as a wheel on sys.path -- so discovery no longer -# depends on the generated tree being reachable as filesystem paths. -# -# This list is maintained by hand, the same way theme packages declare their -# `overture.models` entry points, and changes only when a feature type is -# added or removed. `tests/test_registry.py` fails if it drifts from the tree -# `make generate-pyspark` produces. Values point into that generated tree, -# which is built rather than committed; before it exists an entry point cannot -# load, and the registry skips it (an absent tree yields an empty registry, as -# documented). -[project.entry-points."overture.pyspark_validations"] -address = "overture.schema.pyspark.expressions.generated.overture.schema.addresses.address:MODEL_VALIDATION" -bathymetry = "overture.schema.pyspark.expressions.generated.overture.schema.base.bathymetry:MODEL_VALIDATION" -infrastructure = "overture.schema.pyspark.expressions.generated.overture.schema.base.infrastructure:MODEL_VALIDATION" -land = "overture.schema.pyspark.expressions.generated.overture.schema.base.land:MODEL_VALIDATION" -land_cover = "overture.schema.pyspark.expressions.generated.overture.schema.base.land_cover:MODEL_VALIDATION" -land_use = "overture.schema.pyspark.expressions.generated.overture.schema.base.land_use:MODEL_VALIDATION" -water = "overture.schema.pyspark.expressions.generated.overture.schema.base.water:MODEL_VALIDATION" -building = "overture.schema.pyspark.expressions.generated.overture.schema.buildings.building:MODEL_VALIDATION" -building_part = "overture.schema.pyspark.expressions.generated.overture.schema.buildings.building_part:MODEL_VALIDATION" -division = "overture.schema.pyspark.expressions.generated.overture.schema.divisions.division:MODEL_VALIDATION" -division_area = "overture.schema.pyspark.expressions.generated.overture.schema.divisions.division_area:MODEL_VALIDATION" -division_boundary = "overture.schema.pyspark.expressions.generated.overture.schema.divisions.division_boundary:MODEL_VALIDATION" -place = "overture.schema.pyspark.expressions.generated.overture.schema.places.place:MODEL_VALIDATION" -connector = "overture.schema.pyspark.expressions.generated.overture.schema.transportation.connector:MODEL_VALIDATION" -segment = "overture.schema.pyspark.expressions.generated.overture.schema.transportation.segment:MODEL_VALIDATION" - [tool.uv.build-backend] module-name = "overture.schema.pyspark" diff --git a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py index 6f2d4552d..948277059 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -1,54 +1,31 @@ """Runtime registry of feature validations. -Built at import time from this package's `overture.pyspark_validations` -entry points, one per generated validation module. Reading them through -`importlib.metadata` resolves the same whether the package is installed to a -real directory or loaded straight from a wheel on `sys.path` (as on Glue, via -`--extra-py-files`), so discovery does not depend on the generated tree being -reachable as filesystem paths. - -An entry point whose module is simply absent -- a build without the generated -tree, or a stale declaration -- is skipped, so a package missing its generated -modules still imports cleanly with an empty registry. A module that is present -but fails to import (a missing dependency, a codegen bug) raises, rather than -silently dropping that validation. +Built at import time from the generated `_index` module, which imports every +generated validation module and exposes them as `MODULES`. That import is +ordinary, so the registry populates whether the package is installed to a real +directory or loaded straight from a wheel on `sys.path` (as on Glue, via +`--extra-py-files`); discovery never walks the generated tree as files. + +A build without the generated tree has no `_index` module, so the registry is +empty and the package still imports cleanly. A module that is present but fails +to import (a missing dependency, a codegen bug) raises, so a real breakage is +loud and a validation is never dropped without notice. """ from __future__ import annotations import importlib -import importlib.metadata import logging from .check import ModelValidation logger = logging.getLogger(__name__) -_ENTRY_POINT_GROUP = "overture.pyspark_validations" -_DIST_NAME = "overture-schema-pyspark" - - -def _canonical(name: str) -> str: - """Normalize a distribution name for comparison (PEP 503).""" - return name.replace("_", "-").lower() - - -def _own_entry_points() -> list[importlib.metadata.EntryPoint]: - """Return this distribution's own validation entry points. - - `importlib.metadata.entry_points(group=...)` returns matching entry points - from every installed distribution. Filtering to this one keeps a foreign - package that happens to declare the same group from injecting validations. - """ - return [ - ep - for ep in importlib.metadata.entry_points(group=_ENTRY_POINT_GROUP) - if _canonical(getattr(ep.dist, "name", "") or "") == _DIST_NAME - ] +_INDEX_MODULE = "overture.schema.pyspark.expressions.generated._index" def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: - """Collect registry + partition map from the validation entry points. + """Collect registry + partition map from the generated index. Returns a `(registry, partition_map)` pair: @@ -58,24 +35,25 @@ def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: "place"}`) for path construction. Features with no `PARTITIONS` data (empty dict) are omitted; the codegen only sets `PARTITIONS` when the data lake organizes the feature by Hive partitions. - `type` comes from the entry-point name so consumers get a complete + `type` comes from the module name so consumers get a complete partition path without the codegen having to duplicate the value. """ registry: dict[str, ModelValidation] = {} partition_map: dict[str, dict[str, str]] = {} - for ep in _own_entry_points(): - try: - module = importlib.import_module(ep.module) - except ModuleNotFoundError as e: - missing = e.name or "" - # Skip only when the generated module itself (or an ancestor - # namespace of it) is absent -- a build without the generated tree. - # A dependency missing *inside* a module that is present is a real - # failure; re-raise it instead of silently dropping the validation. - if missing == ep.module or ep.module.startswith(f"{missing}."): - continue - raise + try: + index = importlib.import_module(_INDEX_MODULE) + except ModuleNotFoundError as e: + missing = e.name or "" + # A missing name equal to (or an ancestor of) the index module means + # the generated tree was never built, which is a legitimately empty + # registry. Any other missing name is a real dependency failure while + # importing a module the index references, so let it propagate. + if missing == _INDEX_MODULE or _INDEX_MODULE.startswith(f"{missing}."): + return registry, partition_map + raise + + for module in index.MODULES: entry_point = getattr(module, "ENTRY_POINT", None) validation = getattr(module, "MODEL_VALIDATION", None) if entry_point is None or validation is None: @@ -83,7 +61,8 @@ def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: registry[entry_point] = validation partitions = getattr(module, "PARTITIONS", None) or {} if partitions: - partition_map[entry_point] = {**partitions, "type": ep.name} + feature_type = module.__name__.rsplit(".", 1)[-1] + partition_map[entry_point] = {**partitions, "type": feature_type} return registry, partition_map diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index 04b254ce1..af35732d9 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -1,11 +1,9 @@ """Tests for the runtime registry's discovery of the generated tree. -The registry is built from the `overture.pyspark_validations` entry points, -a hand-maintained table in `pyproject.toml`. These tests hold that table to -the generated tree `make generate-pyspark` produces: one asserts the -registry actually populates, the other that the declared entry points match -the modules on disk exactly, so a forgotten or stale declaration fails here -rather than silently dropping a feature type at runtime. +The registry is built from the generated `_index` module, which codegen emits +alongside the validation modules. One test asserts the registry populates; the +other asserts the index lists exactly the modules on disk, so a codegen bug +that drops a module from the index fails here and never reaches runtime. """ from __future__ import annotations @@ -15,16 +13,18 @@ import pytest -from overture.schema.pyspark._registry import REGISTRY, _own_entry_points +from overture.schema.pyspark._registry import REGISTRY _GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" +_INDEX_MODULE = f"{_GENERATED_ROOT}._index" def _generated_module_names() -> set[str]: """Dotted names of every generated model module on disk. - Empty when the generated tree is absent -- mirroring the registry's own - handling -- so the tests skip rather than error. + Empty when the generated tree is absent, matching the registry's own + handling, so the tests skip cleanly. The `_index` module is excluded + because it is discovery machinery; only validation modules are counted. """ try: root = importlib.import_module(_GENERATED_ROOT) @@ -33,25 +33,15 @@ def _generated_module_names() -> set[str]: names: set[str] = set() for base in root.__path__: for path in Path(base).rglob("*.py"): - if path.name == "__init__.py": + if path.name in ("__init__.py", "_index.py"): continue relative = path.relative_to(base).with_suffix("") names.add(".".join([_GENERATED_ROOT, *relative.parts])) return names -def _declared_entry_point_modules() -> set[str]: - """Modules targeted by this distribution's declared entry points. - - Uses the registry's own filtered lookup so the test checks exactly the - table the registry reads, without picking up a same-named group from - another distribution in the environment. - """ - return {ep.module for ep in _own_entry_points()} - - def test_registry_discovers_generated_models() -> None: - """The registry finds generated modules through the declared entry points.""" + """The registry finds generated modules through the index.""" if not _generated_module_names(): pytest.skip("generated tree not present; run `make generate-pyspark`") @@ -61,21 +51,21 @@ def test_registry_discovers_generated_models() -> None: assert generated_entries, "registry found no generated feature modules" -def test_entry_points_match_generated_tree() -> None: - """Declared entry points match the generated modules on disk exactly. +def test_index_lists_every_generated_module() -> None: + """The generated index covers exactly the modules on disk. - Catches a new feature whose entry point was never declared (present on - disk, missing from the table) and a stale declaration left behind after - a feature was removed (in the table, absent on disk). + Guards the codegen step that emits `_index`: a module generated but left + out of the index (or an index entry with no module) would otherwise drop + that feature type from the registry silently. """ on_disk = _generated_module_names() if not on_disk: pytest.skip("generated tree not present; run `make generate-pyspark`") - declared = _declared_entry_point_modules() - assert declared == on_disk, ( - "overture.pyspark_validations entry points are out of sync with the " - "generated tree.\n" - f" declared but not on disk: {sorted(declared - on_disk)}\n" - f" on disk but not declared: {sorted(on_disk - declared)}" + index = importlib.import_module(_INDEX_MODULE) + indexed = {module.__name__ for module in index.MODULES} + assert indexed == on_disk, ( + "generated _index is out of sync with the modules on disk.\n" + f" indexed but not on disk: {sorted(indexed - on_disk)}\n" + f" on disk but not indexed: {sorted(on_disk - indexed)}" )