[BUG] Make pyspark an optional extra on overture-schema-pyspark - #660
[BUG] Make pyspark an optional extra on overture-schema-pyspark#660Adam Lastowka (Rachmanin0xFF) wants to merge 4 commits into
Conversation
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. Moved it to a spark optional-dependency extra instead. Standalone environments building their own PySpark now install overture-schema-pyspark[spark]; runtime-provided-PySpark environments install the bare package. No other package in this repo depends on overture-schema-pyspark or imports pyspark directly, and CI's make check already runs uv sync --all-extras, so test coverage is unaffected. Signed-off-by: Adam Lastowka <adamlastowka@gmail.com>
GitHub Actions
Inactive
🗺️ Schema reference docs preview is live!
Note ♻️ This preview updates automatically with each push to this PR. |
Victor Schappert (vcschapp)
left a comment
Victor Schappert (vcschapp)
left a comment
There was a problem hiding this comment.
This makes sense to me. Thx Adam Lastowka (@Rachmanin0xFF).
Seth Fitzsimmons (@sethfitz) could you take a look?
|
Adam Lastowka (@Rachmanin0xFF) is this meant to still be in draft mode? |
Victor Schappert (vcschapp)
changed the title
Seth Fitzsimmons (sethfitz)
left a comment
Seth Fitzsimmons (sethfitz)
left a comment
There was a problem hiding this comment.
How do Glue/EMR manage their installed version of PySpark? It seems like “pip” (whichever) should be able to resolve the provided (external to the virtualenv) version, but that probably involves weird uv incantations. Or is it a version conflict?
No objections to doing it this way though.
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Adjusts overture-schema-pyspark packaging so environments that already bundle PySpark (e.g., AWS Glue/EMR) can install without re-resolving a potentially conflicting pyspark wheel.
Changes:
- Moved
pyspark>=3.4from hard dependencies to an optionalsparkextra inpackages/overture-schema-pyspark/pyproject.toml. - Updated
uv.lockto reflect the new optional dependency/extra metadata. - Added a changelog entry documenting the new installation behavior.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| uv.lock | Updates lock metadata so pyspark is only required when the spark extra is selected. |
| packages/overture-schema-pyspark/pyproject.toml | Moves pyspark into [project.optional-dependencies] under the spark extra. |
| packages/overture-schema-pyspark/changelog.d/659.bugfix.md | Documents the dependency change and how to install with/without PySpark. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Victor Schappert (@vcschapp) I was keeping it as a draft because I had Claude vibe-code it while I was in a rush and I hadn't had a chance to actually review it yet. Seth Fitzsimmons (@sethfitz) I don't think it ever reaches |
Importing overture.schema.pyspark without the spark extra installed surfaced a bare ModuleNotFoundError: No module named 'pyspark', with no indication that installing overture-schema-pyspark[spark] fixes it. Guard the package's top-level imports and re-raise with an actionable message pointing at the extra. Narrowed to ModuleNotFoundError on pyspark specifically, so a genuinely broken pyspark install or an unrelated missing dependency still surfaces its own real error. Signed-off-by: Adam Lastowka <adamlastowka@gmail.com>
70db0ab
GitHub Actions
Inactive
| model_names, | ||
| validate_model, | ||
| ) | ||
| except ModuleNotFoundError as e: |
There was a problem hiding this comment.
I think the PySpark imports are a more appropriate place to put this guard, if we go with it.
I had the same initial reaction as Copilot, but talked myself out of it because PySpark is a special snowflake and the whole package effectively depends on it.
Testing the optional dependency at a direct import of pyspark is more precise than wrapping the package's own re-exports and disambiguating the exception by module name: an unrelated missing dependency now surfaces its own error through the real imports instead of being caught and inspected. Per review feedback on the extras change. Signed-off-by: Adam Lastowka <adamlastowka@gmail.com>
Summary
Fixes #659.
overture-schema-pysparkdeclaredpyspark>=3.4as a hard dependency, so installing the package resolvespysparkon every runtime — including ones (Glue, EMR) that already bundle their own PySpark and shouldn't have this package re-resolving a different version underneath it.Change
Moves
pysparkinto[project.optional-dependencies]as asparkextra:pip install overture-schema-pysparknow resolves cleanly against a runtime-provided PySpark. Standalone environments building their own installoverture-schema-pyspark[spark].Verified
overture-schema-pysparkor importspysparkdirectly — nothing else relies on the dependency being unconditional.make check→uv sync --all-packages --all-extrasalready installs all extras, so test coverage is unaffected.uv sync --locked --package overture-schema-pysparkno longer pulls inpyspark.Draft while this gets a look — no urgency on merge timing from my end.