From da22b9b60828cc4a46a5f4c739376b87b1012cfc Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Mon, 17 Aug 2026 13:51:33 -0700 Subject: [PATCH 1/2] fix(pyspark): discover generated modules through importlib.resources `_registry` walked the generated tree with `pathlib.Path.rglob`, which finds nothing when the package is zipimported from a wheel placed on `sys.path` -- what AWS Glue does with `--extra-py-files`. The namespace portion's `__path__` then points inside the archive, `rglob` returns no matches, and the registry comes back empty without raising, so every feature type reports as unregistered and validation silently does nothing. Read the tree through `importlib.resources.files` instead. It resolves a namespace portion whether that portion is a directory or an archive member, and multiplexes every portion, so a tree assembled from more than one distribution is still walked whole. `files()` raises `NotADirectoryError` on a namespace package with any non-directory portion through Python 3.12; the `importlib_resources` backport carries the 3.13 fix, so it is a dependency below 3.13. Glue 4.0 runs Python 3.10 and Glue 5.0 runs 3.11, so on Glue the backport is always the code path taken. Discovery now takes the root package name rather than a list of portion paths, which lets `_walk` drop its own import of the root -- `files()` performs it -- and lets the tests point the walk at a synthetic package. Verified end to end: a wheel on `sys.path`, never unpacked, yields 15 registry entries where the previous implementation yields 0. Signed-off-by: Seth Fitzsimmons --- .../changelog.d/661.bugfix.md | 1 + .../overture-schema-pyspark/pyproject.toml | 3 + .../src/overture/schema/pyspark/_registry.py | 66 +++++++---- .../tests/test_registry.py | 103 +++++++++++++++++- uv.lock | 11 ++ 5 files changed, 159 insertions(+), 25 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..bb378199a --- /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 generated tree is now read through `importlib.resources`, which resolves a namespace portion inside an archive as well as one on disk. diff --git a/packages/overture-schema-pyspark/pyproject.toml b/packages/overture-schema-pyspark/pyproject.toml index 10a190395..9999e2705 100644 --- a/packages/overture-schema-pyspark/pyproject.toml +++ b/packages/overture-schema-pyspark/pyproject.toml @@ -5,6 +5,9 @@ build-backend = "uv_build" [project] dependencies = [ "click>=8.0", + # 6.2 is the first release whose `files()` resolves a namespace portion + # inside a zip; the stdlib gained the same fix in 3.13. + "importlib-resources>=6.2; python_version < '3.13'", "overture-schema-system>=0.1.1", "packaging>=22", ] 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..81268aecc 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -4,18 +4,36 @@ 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 +The generated tree 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. + +The tree is read through `importlib.resources`, which resolves a +namespace portion whether it is a directory on disk or a member of an +archive. A wheel placed straight on `sys.path` is zipimported rather +than installed -- AWS Glue does this with `--extra-py-files` -- and +`pathlib` cannot traverse into one. """ from __future__ import annotations import importlib import logging -from pathlib import Path +import sys +from collections.abc import Iterator + +if sys.version_info >= (3, 13): + from importlib.resources import files + from importlib.resources.abc import Traversable +else: + # `importlib.resources.files` raises `NotADirectoryError` for a namespace + # package with any non-directory portion through Python 3.12; the backport + # carries the 3.13 fix. AWS Glue 4.0 runs Python 3.10 and Glue 5.0 runs + # 3.11, so on Glue this is always the branch taken. + from importlib_resources import files + from importlib_resources.abc import Traversable from .check import ModelValidation @@ -24,23 +42,28 @@ _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. +def _iter_generated_module_names(root: str = _GENERATED_ROOT) -> list[str]: + """Return the dotted names of every generated module under `root`. 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, which `pkgutil.walk_packages` skips. It is walked + as resources instead: every `.py` below `root`, keyed to a dotted name. + `files` multiplexes every portion of the namespace, so a tree assembled + from more than one distribution is walked whole. """ - 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 + + def walk(node: Traversable, prefix: tuple[str, ...]) -> Iterator[str]: + for child in node.iterdir(): + if child.is_dir(): + yield from walk(child, (*prefix, child.name)) + elif child.name.endswith(".py") and child.name != "__init__.py": + yield ".".join([root, *prefix, child.name[: -len(".py")]]) + + try: + anchor = files(root) + except ModuleNotFoundError: + return [] + return sorted(walk(anchor, ())) def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: @@ -61,12 +84,7 @@ def _walk() -> tuple[dict[str, ModelValidation], dict[str, dict[str, str]]]: 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__)): + for name in _iter_generated_module_names(): module = importlib.import_module(name) entry_point = getattr(module, "ENTRY_POINT", None) validation = getattr(module, "MODEL_VALIDATION", None) diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index fcf261207..aed94e6a6 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -5,19 +5,85 @@ 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 walk has to work for a namespace portion inside a zip as well as one on +disk, because a wheel placed straight on `sys.path` is zipimported rather +than installed (AWS Glue's `--extra-py-files`). Both shapes are exercised +here against a synthetic package put on `sys.path`, so the portion strings +under test come from real import machinery rather than being hand-written. """ from __future__ import annotations import importlib +import sys +import zipfile +from collections.abc import Iterator, Sequence +from contextlib import contextmanager from pathlib import Path import pytest -from overture.schema.pyspark._registry import REGISTRY +from overture.schema.pyspark._registry import REGISTRY, _iter_generated_module_names _GENERATED_ROOT = "overture.schema.pyspark.expressions.generated" +_PROBE_TOP = "zipimport_probe" +_PROBE_ROOT = f"{_PROBE_TOP}.expressions.generated" + + +@contextmanager +def _on_sys_path(entry: Path) -> Iterator[None]: + """Put `entry` on `sys.path` and unimport the probe package after.""" + sys.path.insert(0, str(entry)) + importlib.invalidate_caches() + try: + yield + finally: + sys.path.remove(str(entry)) + for name in [n for n in sys.modules if n.split(".")[0] == _PROBE_TOP]: + del sys.modules[name] + importlib.invalidate_caches() + + +def _probe_members(relative_paths: Sequence[str]) -> list[str]: + """Return archive member names for a probe tree holding `relative_paths`.""" + return [f"{_PROBE_ROOT.replace('.', '/')}/{p}" for p in relative_paths] + + +def _write_probe_wheel(wheel: Path, relative_paths: Sequence[str]) -> None: + """Write a wheel-shaped zip holding a PEP 420 probe tree. + + Directory entries are written explicitly. `zipimport` recognises a + namespace portion inside an archive only when the archive carries a + directory entry for it, and that is what `uv_build` emits, so omitting + them here would test a shape no real wheel has. + """ + members = _probe_members(relative_paths) + directories = sorted( + { + f"{parent}/" + for member in members + for parent in (str(p) for p in Path(member).parents) + if parent != "." + } + ) + with zipfile.ZipFile(wheel, "w") as archive: + for directory in directories: + info = zipfile.ZipInfo(directory) + info.external_attr = (0o40755 << 16) | 0x10 + archive.writestr(info, b"") + for member in members: + archive.writestr(member, "") + + +def _write_probe_directory(root: Path, relative_paths: Sequence[str]) -> None: + """Write a PEP 420 probe tree as real directories under `root`.""" + for member in _probe_members(relative_paths): + path = root / member + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("") + def _generated_leaf_count() -> int: """Count generated model modules on disk (excludes namespace dirs). @@ -46,3 +112,38 @@ 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_a_zipimported_tree(tmp_path: Path) -> None: + """Modules inside a wheel on `sys.path` are discovered. + + Mirrors Glue's `--extra-py-files`: the wheel is never unpacked, so the + namespace portion `__path__` points inside the archive. + """ + wheel = tmp_path / "zipimport_probe-0.0.0-py3-none-any.whl" + _write_probe_wheel(wheel, ["schema/base/water.py", "schema/buildings/building.py"]) + + with _on_sys_path(wheel): + names = _iter_generated_module_names(_PROBE_ROOT) + + assert names == [ + f"{_PROBE_ROOT}.schema.base.water", + f"{_PROBE_ROOT}.schema.buildings.building", + ] + + +def test_iter_generated_module_names_reads_a_directory_tree(tmp_path: Path) -> None: + """Modules in a real directory are discovered, and `__init__.py` is skipped.""" + _write_probe_directory( + tmp_path, ["schema/base/water.py", "schema/base/__init__.py"] + ) + + with _on_sys_path(tmp_path): + names = _iter_generated_module_names(_PROBE_ROOT) + + assert names == [f"{_PROBE_ROOT}.schema.base.water"] + + +def test_iter_generated_module_names_without_a_tree_is_empty() -> None: + """An absent generated tree yields no names rather than raising.""" + assert _iter_generated_module_names(f"{_PROBE_ROOT}.absent") == [] diff --git a/uv.lock b/uv.lock index 33b465975..0b918f474 100644 --- a/uv.lock +++ b/uv.lock @@ -379,6 +379,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/1e/5e/d4e9f1a599fb8e573b7b87160658329fbf28d19eac2718f51fc3def3aa5a/idna-3.18-py3-none-any.whl", hash = "sha256:7f952cbe720b688055e3f87de14f5c3e5fdaa8bc3928985c4077ca689de849a2", size = 65455, upload-time = "2026-06-02T14:34:06.319Z" }, ] +[[package]] +name = "importlib-resources" +version = "7.1.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e4/06/b56dfa750b44e86157093bc8fca0ab81dccbf5260510de4eaf1cb69b5b99/importlib_resources-7.1.0.tar.gz", hash = "sha256:0722d4c6212489c530f2a145a34c0a7a3b4721bc96a15fada5930e2a0b760708", size = 44985, upload-time = "2026-04-12T16:36:09.232Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/8a/db/55a262f3606bebcae07cc14095338471ad7c0bbcaa37707e6f0ee49725b7/importlib_resources-7.1.0-py3-none-any.whl", hash = "sha256:1bd7b48b4088eddb2cd16382150bb515af0bd2c70128194392725f82ad2c96a1", size = 37232, upload-time = "2026-04-12T16:36:08.219Z" }, +] + [[package]] name = "iniconfig" version = "2.3.0" @@ -1041,6 +1050,7 @@ version = "0.1.1" source = { editable = "packages/overture-schema-pyspark" } dependencies = [ { name = "click" }, + { name = "importlib-resources", marker = "python_full_version < '3.13'" }, { name = "overture-schema-system" }, { name = "packaging" }, ] @@ -1053,6 +1063,7 @@ spark = [ [package.metadata] requires-dist = [ { name = "click", specifier = ">=8.0" }, + { name = "importlib-resources", marker = "python_full_version < '3.13'", specifier = ">=6.2" }, { name = "overture-schema-system", editable = "packages/overture-schema-system" }, { name = "packaging", specifier = ">=22" }, { name = "pyspark", marker = "extra == 'spark'", specifier = ">=3.4" }, From 79059325de532baee59ea936feb2d09661d0d142 Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Mon, 17 Aug 2026 14:10:27 -0700 Subject: [PATCH 2/2] docs(pyspark): describe the zipimport failure without pinning it to Glue `--extra-py-files` is Glue's job parameter, not Spark's -- `spark-submit` takes `--py-files` and knows nothing by that name. Naming Glue as the cause made a general property read like a vendor quirk: any wheel left unextracted on `sys.path` is zipimported, which is what Spark's `--py-files` and `SparkContext.addPyFile` produce, and what a bare `PYTHONPATH` entry produces with no Spark involved at all. Lead with the mechanism and keep Glue as an instance of it. The version gate's comment still names the Glue runtimes below the 3.13 stdlib fix, since those are evidence for the gate rather than an attribution, and now invites readers to add other runtimes they find below it. Signed-off-by: Seth Fitzsimmons --- .../src/overture/schema/pyspark/_registry.py | 12 +++++++----- .../overture-schema-pyspark/tests/test_registry.py | 13 +++++++------ 2 files changed, 14 insertions(+), 11 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 81268aecc..4c93e2f3d 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py @@ -12,9 +12,9 @@ The tree is read through `importlib.resources`, which resolves a namespace portion whether it is a directory on disk or a member of an -archive. A wheel placed straight on `sys.path` is zipimported rather -than installed -- AWS Glue does this with `--extra-py-files` -- and -`pathlib` cannot traverse into one. +archive. Any wheel left unextracted on `sys.path` is zipimported and +`pathlib` cannot traverse into it -- Spark ships wheels that way with +`--py-files`, as does AWS Glue with `--extra-py-files`. """ from __future__ import annotations @@ -30,8 +30,10 @@ else: # `importlib.resources.files` raises `NotADirectoryError` for a namespace # package with any non-directory portion through Python 3.12; the backport - # carries the 3.13 fix. AWS Glue 4.0 runs Python 3.10 and Glue 5.0 runs - # 3.11, so on Glue this is always the branch taken. + # carries the 3.13 fix. Runtimes known to sit below it, where this is always + # the branch taken: AWS Glue 4.0 (Python 3.10) and Glue 5.0 (3.11). Other + # runtimes that ship wheels unextracted belong on that list -- if you hit + # this somewhere else, please add what you find. from importlib_resources import files from importlib_resources.abc import Traversable diff --git a/packages/overture-schema-pyspark/tests/test_registry.py b/packages/overture-schema-pyspark/tests/test_registry.py index aed94e6a6..733eed624 100644 --- a/packages/overture-schema-pyspark/tests/test_registry.py +++ b/packages/overture-schema-pyspark/tests/test_registry.py @@ -7,10 +7,11 @@ registry would otherwise pass the suite unnoticed. The walk has to work for a namespace portion inside a zip as well as one on -disk, because a wheel placed straight on `sys.path` is zipimported rather -than installed (AWS Glue's `--extra-py-files`). Both shapes are exercised -here against a synthetic package put on `sys.path`, so the portion strings -under test come from real import machinery rather than being hand-written. +disk, because any wheel left unextracted on `sys.path` is zipimported -- +Spark's `--py-files` and Glue's `--extra-py-files` both ship wheels that way. +Both shapes are exercised here against a synthetic package put on `sys.path`, +so the portion strings under test come from real import machinery rather than +being hand-written. """ from __future__ import annotations @@ -117,8 +118,8 @@ def test_registry_discovers_generated_models() -> None: def test_iter_generated_module_names_reads_a_zipimported_tree(tmp_path: Path) -> None: """Modules inside a wheel on `sys.path` are discovered. - Mirrors Glue's `--extra-py-files`: the wheel is never unpacked, so the - namespace portion `__path__` points inside the archive. + The wheel is never unpacked, so the namespace portion's `__path__` points + inside the archive. """ wheel = tmp_path / "zipimport_probe-0.0.0-py3-none-any.whl" _write_probe_wheel(wheel, ["schema/base/water.py", "schema/buildings/building.py"])