From 12806c14cdd7376a4c0469199e4716e2b87018ce Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Wed, 19 Aug 2026 10:50:22 -0700 Subject: [PATCH 1/2] fix(pyspark): make pyspark an optional extra pyspark was a hard dependency, so installing overture-schema-pyspark resolved pyspark on every runtime, including ones (Glue, EMR) that already bundle their own PySpark and don't want this package re-resolving it underneath them. Moved it to a `spark` optional-dependency extra. Standalone environments building their own install overture-schema-pyspark[spark]; runtime-provided-PySpark environments install the bare package. Probe for pyspark in the package's __init__ so a bare install without PySpark raises an actionable ModuleNotFoundError naming the extra rather than "No module named 'pyspark'". __init__ runs on any import of any submodule, generated ones included, so one probe covers every entry path. It catches nothing, so an unrelated missing dependency -- or a pyspark that is present but broken -- still surfaces its own real error from the imports that follow. Supersedes #660, which is where this change and its changelog wording originated. Refs #659 Co-authored-by: Adam Lastowka Signed-off-by: Seth Fitzsimmons --- .../changelog.d/659.bugfix.md | 1 + .../overture-schema-pyspark/pyproject.toml | 4 +- .../src/overture/schema/pyspark/__init__.py | 15 ++++ .../tests/test_optional_pyspark.py | 75 +++++++++++++++++++ uv.lock | 7 +- 5 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 packages/overture-schema-pyspark/changelog.d/659.bugfix.md create mode 100644 packages/overture-schema-pyspark/tests/test_optional_pyspark.py diff --git a/packages/overture-schema-pyspark/changelog.d/659.bugfix.md b/packages/overture-schema-pyspark/changelog.d/659.bugfix.md new file mode 100644 index 000000000..133bc11dd --- /dev/null +++ b/packages/overture-schema-pyspark/changelog.d/659.bugfix.md @@ -0,0 +1 @@ +Moved `pyspark` from a hard dependency to the `spark` optional extra, so installing `overture-schema-pyspark` no longer resolves `pyspark` on runtimes (Glue, EMR) that already bundle their own. Standalone environments need `pip install overture-schema-pyspark[spark]`. Importing the package without PySpark installed now raises an actionable `ModuleNotFoundError` pointing at that extra, instead of a bare "No module named 'pyspark'". diff --git a/packages/overture-schema-pyspark/pyproject.toml b/packages/overture-schema-pyspark/pyproject.toml index 6e72bc93c..a280bf72c 100644 --- a/packages/overture-schema-pyspark/pyproject.toml +++ b/packages/overture-schema-pyspark/pyproject.toml @@ -6,7 +6,6 @@ build-backend = "uv_build" dependencies = [ "click>=8.0", "overture-schema-system>=0.1.1", - "pyspark>=3.4", ] description = "PySpark validation expressions for Overture Maps data" license = "MIT" @@ -14,6 +13,9 @@ name = "overture-schema-pyspark" requires-python = ">=3.10" version = "0.1.1" +[project.optional-dependencies] +spark = ["pyspark>=3.4"] + [project.scripts] overture-validate = "overture.schema.pyspark.cli:validate_cli" diff --git a/packages/overture-schema-pyspark/src/overture/schema/pyspark/__init__.py b/packages/overture-schema-pyspark/src/overture/schema/pyspark/__init__.py index 8841ae9f4..1bc7b3b69 100644 --- a/packages/overture-schema-pyspark/src/overture/schema/pyspark/__init__.py +++ b/packages/overture-schema-pyspark/src/overture/schema/pyspark/__init__.py @@ -1,5 +1,20 @@ """PySpark validation expressions for Overture Maps data.""" +import importlib.util + +# pyspark is an optional extra (the `spark` extra): a bare install lets this +# package's metadata and console script resolve, but every module below needs +# pyspark itself to do anything. Probe for it here -- this module runs on any +# import of any submodule -- so a bare install gets an actionable message. A +# pyspark that is present but broken still raises its own error, because the +# imports below run for real. +if importlib.util.find_spec("pyspark") is None: + raise ModuleNotFoundError( + "overture-schema-pyspark requires PySpark, which isn't installed. " + "Install it with `pip install overture-schema-pyspark[spark]`, or run " + "in an environment that already provides PySpark (e.g. a Spark cluster)." + ) + from .check import Check, CheckShape from .schema_check import SchemaMismatch, compare_schemas from .validate import ( diff --git a/packages/overture-schema-pyspark/tests/test_optional_pyspark.py b/packages/overture-schema-pyspark/tests/test_optional_pyspark.py new file mode 100644 index 000000000..fe1d2be21 --- /dev/null +++ b/packages/overture-schema-pyspark/tests/test_optional_pyspark.py @@ -0,0 +1,75 @@ +"""Tests for the missing-PySpark guard in the package's __init__.""" + +import importlib +import sys +from types import ModuleType + +import pytest + + +class _BlockedFinder: + """A meta_path finder that reports one module as genuinely absent. + + Raising from `find_spec` reproduces what an uninstalled module does at an + `import` statement -- a `ModuleNotFoundError` carrying `name`. Setting + `sys.modules[name] = None` would raise `ImportError` instead, which is not + the error a missing package actually produces. + """ + + def __init__(self, name: str) -> None: + self.name = name + + def find_spec( + self, fullname: str, path: object = None, target: ModuleType | None = None + ) -> None: + if fullname == self.name or fullname.startswith(f"{self.name}."): + raise ModuleNotFoundError(f"No module named {fullname!r}", name=fullname) + return None + + +def _unload(monkeypatch: pytest.MonkeyPatch, *roots: str) -> None: + """Drop modules from sys.modules so importing them re-runs their top level. + + monkeypatch restores the original module objects at teardown, so modules + already imported by the rest of the suite keep their identity. + """ + for name in [ + name + for name in sys.modules + if any(name == root or name.startswith(f"{root}.") for root in roots) + ]: + monkeypatch.delitem(sys.modules, name) + + +def test_missing_pyspark_names_the_extra(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setitem(sys.modules, "pyspark", None) + _unload(monkeypatch, "overture.schema.pyspark") + + with pytest.raises(ModuleNotFoundError) as excinfo: + importlib.import_module("overture.schema.pyspark") + + assert "overture-schema-pyspark[spark]" in str(excinfo.value) + + +def test_unrelated_missing_dependency_surfaces_its_own_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The guard reports only a missing pyspark, never another package's.""" + _unload(monkeypatch, "overture.schema.pyspark", "overture.schema.system", "shapely") + monkeypatch.setattr(sys, "meta_path", [_BlockedFinder("shapely"), *sys.meta_path]) + + with pytest.raises(ImportError) as excinfo: + importlib.import_module("overture.schema.pyspark") + + assert "shapely" in str(excinfo.value) + assert "overture-schema-pyspark[spark]" not in str(excinfo.value) + + +def test_package_imports_when_pyspark_is_installed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _unload(monkeypatch, "overture.schema.pyspark") + + module = importlib.import_module("overture.schema.pyspark") + + assert module.Check.__name__ == "Check" diff --git a/uv.lock b/uv.lock index 5e0bcfd1a..a5a248a3f 100644 --- a/uv.lock +++ b/uv.lock @@ -1042,6 +1042,10 @@ source = { editable = "packages/overture-schema-pyspark" } dependencies = [ { name = "click" }, { name = "overture-schema-system" }, +] + +[package.optional-dependencies] +spark = [ { name = "pyspark" }, ] @@ -1049,8 +1053,9 @@ dependencies = [ requires-dist = [ { name = "click", specifier = ">=8.0" }, { name = "overture-schema-system", editable = "packages/overture-schema-system" }, - { name = "pyspark", specifier = ">=3.4" }, + { name = "pyspark", marker = "extra == 'spark'", specifier = ">=3.4" }, ] +provides-extras = ["spark"] [[package]] name = "overture-schema-system" From de17a45a6d6ca3eb5276847907b76fdc64e0d66f Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Wed, 19 Aug 2026 11:20:28 -0700 Subject: [PATCH 2/2] review: stage proposed upstream PR body Signed-off-by: Seth Fitzsimmons --- .pr-review/BODY.md | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .pr-review/BODY.md diff --git a/.pr-review/BODY.md b/.pr-review/BODY.md new file mode 100644 index 000000000..6f91778a7 --- /dev/null +++ b/.pr-review/BODY.md @@ -0,0 +1,47 @@ +Closes #659. Supersedes #660, which is where this change and its changelog wording originated; @Rachmanin0xFF is credited as co-author on the commit. + +## Problem + +`overture-schema-pyspark` declared `pyspark>=3.4` as a hard dependency, so installing the package resolved `pyspark` on every runtime — including ones (Glue, EMR) that already bundle their own PySpark and shouldn't have this package re-resolving a different version underneath them. + +## Change + +`pyspark` moves to an optional extra: + +```toml +[project.optional-dependencies] +spark = ["pyspark>=3.4"] +``` + +`pip install overture-schema-pyspark` now resolves cleanly against a runtime-provided PySpark. Standalone environments building their own install `overture-schema-pyspark[spark]`. + +That makes "PySpark isn't here" an expected first-run state rather than a broken install, so the package says what to do about it. `overture/schema/pyspark/__init__.py` probes for pyspark before its own imports: + +```python +if importlib.util.find_spec("pyspark") is None: + raise ModuleNotFoundError( + "overture-schema-pyspark requires PySpark, which isn't installed. " + "Install it with `pip install overture-schema-pyspark[spark]`, or run " + "in an environment that already provides PySpark (e.g. a Spark cluster)." + ) +``` + +Two properties are worth stating, because they're what make one probe sufficient: + +**It covers every entry path.** `overture/schema/pyspark/__init__.py` is a real `__init__.py` — only `overture` and `overture.schema` are PEP 420 namespaces — so it executes on any import of any submodule, including the 15 generated expression modules that import `pyspark.sql` directly. Guarding at those import sites instead would mean 22 guards (7 hand-written modules plus the generated tree, which would need a renderer change), of which only one is ever reachable: the re-export chain always hits `check.py` first. + +**It catches nothing.** A `try`/`except ModuleNotFoundError` around the imports would also swallow an unrelated missing dependency, and — less obviously — would misreport an installed-but-broken pyspark. With `py4j` removed, `import pyspark` raises `ModuleNotFoundError(name='py4j')`; an `except` clause reports "PySpark isn't installed", which is false, and avoiding that requires inspecting `e.name`. The probe has no such branch: it asks whether pyspark is findable, and everything else surfaces its own real error from the imports that follow. + +## Tests + +`tests/test_optional_pyspark.py` adds three tests. One is the gate: `test_missing_pyspark_names_the_extra` fails against the unguarded `__init__` and passes with the probe. The other two are guards, and pass both before and after — `test_unrelated_missing_dependency_surfaces_its_own_error` pins the no-masking property (verified to fail against a broad-`except` implementation), and `test_package_imports_when_pyspark_is_installed` pins the normal path. + +The two blocking mechanisms differ on purpose. The pyspark test sets `sys.modules["pyspark"] = None`, which makes `find_spec` return `None` — the same signal real absence produces at the seam this code reads. The unrelated-dependency test uses a `meta_path` finder that raises `ModuleNotFoundError(..., name=...)`, because `sys.modules[x] = None` raises `ImportError` instead, and a control built on it would pass under the very implementation it exists to forbid. + +## Verified + +- `make check TESTMON=` — 6201 passed. +- Bare install in a clean venv (`uv pip install ./packages/overture-schema-system ./packages/overture-schema-pyspark`, no pyspark): importing the package raises the actionable message; the automated test pins the branch, this pins that `find_spec` is the right question. +- Same venv with `shapely` also removed, and the worktree venv with `py4j` removed: both surface their own errors, unmasked. +- `uv sync --locked --all-packages --all-extras` accepts the lockfile; CI's `make check` installs all extras, so coverage is unaffected. +- No other package in this repo depends on `overture-schema-pyspark` or imports `pyspark`.