From 23988388b2354ed5c544c8afbb388a1633869e88 Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Thu, 13 Aug 2026 21:48:58 -0400 Subject: [PATCH 1/4] Fix validation registry discovery under zipimport REGISTRY was built by walking expressions/generated/ with pathlib.Path.rglob, which only sees real filesystem directories. When overture-schema-pyspark is loaded straight from a wheel on sys.path (zipimport) instead of being extracted -- as AWS Glue does via --extra-py-files -- the namespace package's __path__ portions point inside the zip archive, and pathlib can't traverse into one. The walk silently found nothing, so validate_model()/get_feature_validation() reported every feature type as unregistered. importlib.resources.files() looks like the fix, since its Traversable API is meant to be zipimport-aware, but its MultiplexedPath implementation (through at least Python 3.10) raises NotADirectoryError the moment any namespace portion isn't a real directory -- confirmed against a zip-imported wheel before ruling it out. Walk each namespace portion directly instead: pathlib for real directories (unchanged), zipfile.ZipFile.namelist() for portions that resolve to a path inside a zip file. Verified against a real wheel added to sys.path without extraction (all 15 generated modules now resolve), the existing overture-schema-pyspark test suite (3132 passed), and the documented empty-registry behavior when expressions/generated/ is absent entirely (via make clean-pyspark). Fixes #661 Signed-off-by: Adam Lastowka --- .../changelog.d/661.bugfix.md | 1 + .../src/overture/schema/pyspark/_registry.py | 61 +++++++++++++++---- 2 files changed, 51 insertions(+), 11 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..1136a8313 --- /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`. Generated modules under `expressions/generated/` are now discovered whether the package lives on disk or inside a zip archive. 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..373e69e2a 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -15,7 +15,8 @@ import importlib import logging -from pathlib import Path +import zipfile +from pathlib import Path, PurePosixPath from .check import ModelValidation @@ -24,23 +25,61 @@ _GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" +def _zip_boundary(root_path: str) -> tuple[str, str] | None: + """Split a namespace portion into `(zip file path, internal prefix)`. + + Returns `None` if `root_path` isn't a real directory and isn't inside a + zip either (e.g. an empty/nonexistent portion). A namespace portion + loaded straight from a wheel on `sys.path` -- as happens on Glue, via + `--extra-py-files` -- looks like `.../some_pkg-1.0-py3-none-any.whl/a/b/c`: + not a directory on disk, but the leading `.../some_pkg....whl` segment is + a real zip file. `importlib.resources`'s `Traversable` API is meant to + cover exactly this, but its `MultiplexedPath` (at least through Python + 3.10) raises `NotADirectoryError` the moment any namespace portion isn't + a real directory, so it can't be used here either. + """ + path = Path(root_path) + for parent in (path, *path.parents): + if parent.is_file() and zipfile.is_zipfile(parent): + return str(parent), path.relative_to(parent).as_posix() + return None + + def _iter_generated_module_names(root_paths: list[str]) -> list[str]: - """Return the dotted names of every generated module on disk. + """Return the dotted names of every generated module under `root_paths`. 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`. + are namespace packages; `pkgutil.walk_packages` skips those, so each + namespace portion is walked directly instead: as a real directory via + `pathlib`, or as a zip member list via `zipfile` when the portion is + inside a wheel on `sys.path` rather than extracted to disk. """ 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 + if base.is_dir(): + 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])) + continue + + boundary = _zip_boundary(root_path) + if boundary is None: + continue + zip_path, prefix = boundary + prefix = f"{prefix}/" if prefix else "" + with zipfile.ZipFile(zip_path) as archive: + for entry in archive.namelist(): + if not entry.startswith(prefix) or not entry.endswith(".py"): + continue + relative = PurePosixPath(entry[len(prefix) :]) + if relative.name == "__init__.py": + continue + dotted = relative.with_suffix("").as_posix().replace("/", ".") + names.append(".".join([_GENERATED_ROOT, dotted])) + return sorted(names) def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: From 67cc5a7e23fed43a820407682d51bee59a4a8b4d Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 02:44:36 -0400 Subject: [PATCH 2/4] Cover the zip branch of registry discovery The on-disk walk had incidental coverage but the zip branch (_zip_boundary + ZipFile.namelist) had none: the existing test skips under zipimport, so a regression in the wheel-on-sys.path path would go unnoticed. Exercise it directly against a synthetic wheel, asserting namespace __init__.py markers and members outside the generated prefix are excluded, plus a non-zip path yields nothing. Signed-off-by: Adam Lastowka --- .../tests/test_registry.py | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index fcf261207..bd31fb240 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -5,18 +5,25 @@ 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 zip branch of the walk (a namespace portion inside a wheel on +`sys.path`, as on Glue) has no such incidental coverage either, so it is +exercised directly here against a synthetic archive. """ from __future__ import annotations import importlib +import zipfile from pathlib import Path import pytest -from overture.schema.pyspark._registry import REGISTRY - -_GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" +from overture.schema.pyspark._registry import ( + _GENERATED_ROOT, + REGISTRY, + _iter_generated_module_names, +) def _generated_leaf_count() -> int: @@ -46,3 +53,33 @@ def test_registry_discovers_generated_models() -> None: key for key in REGISTRY if ":" in key and key.startswith("overture.schema.") ] assert generated_entries, "registry found no generated feature modules on disk" + + +def test_iter_generated_module_names_reads_zip(tmp_path: Path) -> None: + """The zip branch lists generated modules inside a wheel on `sys.path`. + + Mirrors how Glue loads the package via `--extra-py-files`: the namespace + portion is `/overture/schema/pyspark/expressions/generated`, whose + leading segment is a real zip file rather than a directory. Namespace + `__init__.py` markers and members outside the generated prefix are + excluded, and the two feature modules come back as dotted names. + """ + prefix = "overture/schema/pyspark/expressions/generated" + wheel = tmp_path / "overture_schema_pyspark-0.0.0-py3-none-any.whl" + with zipfile.ZipFile(wheel, "w") as archive: + archive.writestr(f"{prefix}/overture/schema/buildings/building.py", "\n") + archive.writestr(f"{prefix}/overture/schema/base/water.py", "\n") + archive.writestr(f"{prefix}/overture/schema/base/__init__.py", "\n") + archive.writestr("overture/schema/pyspark/check.py", "\n") + + names = _iter_generated_module_names([f"{wheel}/{prefix}"]) + + assert names == [ + f"{_GENERATED_ROOT}.overture.schema.base.water", + f"{_GENERATED_ROOT}.overture.schema.buildings.building", + ] + + +def test_iter_generated_module_names_ignores_nonexistent_path(tmp_path: Path) -> None: + """A portion that is neither a directory nor inside a zip yields nothing.""" + assert _iter_generated_module_names([str(tmp_path / "missing")]) == [] From fe83cdbc96c0d03bccdb071da1e268eb29d50505 Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 02:53:14 -0400 Subject: [PATCH 3/4] Fix mypy type collision in the zip walk branch The directory branch binds `relative` to a `Path`; the zip branch reused the same name for a `PurePosixPath`, which mypy rejects as an incompatible reassignment. Give the zip member its own variable. Signed-off-by: Adam Lastowka --- .../src/overture/schema/pyspark/_registry.py | 6 +++--- 1 file changed, 3 insertions(+), 3 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 373e69e2a..84a9f2119 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -74,10 +74,10 @@ def _iter_generated_module_names(root_paths: list[str]) -> list[str]: for entry in archive.namelist(): if not entry.startswith(prefix) or not entry.endswith(".py"): continue - relative = PurePosixPath(entry[len(prefix) :]) - if relative.name == "__init__.py": + member = PurePosixPath(entry[len(prefix) :]) + if member.name == "__init__.py": continue - dotted = relative.with_suffix("").as_posix().replace("/", ".") + dotted = member.with_suffix("").as_posix().replace("/", ".") names.append(".".join([_GENERATED_ROOT, dotted])) return sorted(names) From c9c2f8407a3c5ed4f3a6ce4e24784e88a28a49e4 Mon Sep 17 00:00:00 2001 From: Adam Lastowka Date: Fri, 14 Aug 2026 03:21:03 -0400 Subject: [PATCH 4/4] Reword registry docstrings for house style Drop the em-dash asides and contrastive phrasing from the docstrings added for the zip-walk fix, matching the comment conventions used elsewhere in this contribution. No behaviour change. Signed-off-by: Adam Lastowka --- .../src/overture/schema/pyspark/_registry.py | 29 ++++++++++--------- .../tests/test_registry.py | 18 ++++++------ 2 files changed, 24 insertions(+), 23 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 84a9f2119..41f5d1b61 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -7,7 +7,7 @@ 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 -- +`expressions/generated/` subtree simply yields an empty registry, and the package still imports cleanly. """ @@ -28,15 +28,15 @@ def _zip_boundary(root_path: str) -> tuple[str, str] | None: """Split a namespace portion into `(zip file path, internal prefix)`. - Returns `None` if `root_path` isn't a real directory and isn't inside a - zip either (e.g. an empty/nonexistent portion). A namespace portion - loaded straight from a wheel on `sys.path` -- as happens on Glue, via - `--extra-py-files` -- looks like `.../some_pkg-1.0-py3-none-any.whl/a/b/c`: - not a directory on disk, but the leading `.../some_pkg....whl` segment is - a real zip file. `importlib.resources`'s `Traversable` API is meant to - cover exactly this, but its `MultiplexedPath` (at least through Python - 3.10) raises `NotADirectoryError` the moment any namespace portion isn't - a real directory, so it can't be used here either. + Returns `None` when `root_path` is neither a real directory nor inside a + zip, such as an empty or nonexistent portion. When Glue loads the package + from a wheel via `--extra-py-files`, a namespace portion looks like + `.../some_pkg-1.0-py3-none-any.whl/a/b/c`, where the leading + `.../some_pkg....whl` segment is a real zip file even though the whole + path is not a directory on disk. `importlib.resources`'s `Traversable` + API is meant to cover this, but its `MultiplexedPath` (at least through + Python 3.10) raises `NotADirectoryError` the moment any namespace portion + is not a real directory, so it does not work here. """ path = Path(root_path) for parent in (path, *path.parents): @@ -49,10 +49,11 @@ def _iter_generated_module_names(root_paths: list[str]) -> list[str]: """Return the dotted names of every generated module under `root_paths`. The generated tree is PEP 420 (no `__init__.py`), so its subdirectories - are namespace packages; `pkgutil.walk_packages` skips those, so each - namespace portion is walked directly instead: as a real directory via - `pathlib`, or as a zip member list via `zipfile` when the portion is - inside a wheel on `sys.path` rather than extracted to disk. + are namespace packages, which `pkgutil.walk_packages` skips. Each + namespace portion is therefore walked directly. A portion that is a real + directory is walked with `pathlib`; one that sits inside a wheel on + `sys.path`, as under Glue's `--extra-py-files`, is read from the archive + with `zipfile`. """ names: list[str] = [] for root_path in root_paths: diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index bd31fb240..2dfd00dd5 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -2,9 +2,9 @@ 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. +real on-disk walk, since 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 zip branch of the walk (a namespace portion inside a wheel on `sys.path`, as on Glue) has no such incidental coverage either, so it is @@ -29,8 +29,8 @@ def _generated_leaf_count() -> int: """Count generated model modules on disk (excludes namespace dirs). - Returns 0 when the generated tree is absent -- mirroring the registry's - own `ImportError` handling -- so the test skips rather than errors. + Returns 0 when the generated tree is absent, matching the registry's own + `ImportError` handling, so the test skips cleanly. """ try: root = importlib.import_module(_GENERATED_ROOT) @@ -59,10 +59,10 @@ def test_iter_generated_module_names_reads_zip(tmp_path: Path) -> None: """The zip branch lists generated modules inside a wheel on `sys.path`. Mirrors how Glue loads the package via `--extra-py-files`: the namespace - portion is `/overture/schema/pyspark/expressions/generated`, whose - leading segment is a real zip file rather than a directory. Namespace - `__init__.py` markers and members outside the generated prefix are - excluded, and the two feature modules come back as dotted names. + portion is `/overture/schema/pyspark/expressions/generated`, and + its leading `` segment is a real zip file. Namespace `__init__.py` + markers and members outside the generated prefix are excluded, and the + two feature modules come back as dotted names. """ prefix = "overture/schema/pyspark/expressions/generated" wheel = tmp_path / "overture_schema_pyspark-0.0.0-py3-none-any.whl"