[BUG] Fix validation registry discovery when loaded via zipimport - #662
Open
Adam Lastowka (Rachmanin0xFF) wants to merge 4 commits into
Open
[BUG] Fix validation registry discovery when loaded via zipimport#662Adam Lastowka (Rachmanin0xFF) wants to merge 4 commits into
Adam Lastowka (Rachmanin0xFF) wants to merge 4 commits into
Conversation
Adam Lastowka (Rachmanin0xFF)
added
the
change type - minor 🤏
Copilot started reviewing on behalf of
Adam Lastowka (Rachmanin0xFF)
August 14, 2026 01:49
View session
Adam Lastowka (Rachmanin0xFF)
temporarily deployed
to
staging
August 14, 2026 01:50 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
🗺️ Schema reference docs preview is live!
Note ♻️ This preview updates automatically with each push to this PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a bug where overture.schema.pyspark._registry.REGISTRY stays empty when overture-schema-pyspark is imported from a wheel on sys.path (zipimport, e.g. AWS Glue --extra-py-files) by making generated-module discovery work for both filesystem and zip-backed namespace portions.
Changes:
- Add zip-aware namespace portion discovery by detecting the “zip boundary” and listing
.pymembers viazipfile.ZipFile.namelist(). - Preserve existing behavior for real on-disk directories using
pathlib.Path(...).rglob("*.py"). - Add a changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/overture-schema-pyspark/src/overture/schema/pyspark/_registry.py | Implements zipimport-aware generated-module discovery for namespace package portions inside wheels. |
| packages/overture-schema-pyspark/changelog.d/661.bugfix.md | Documents the bugfix for empty registry under zipimport (AWS Glue-style loading). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Adam Lastowka (Rachmanin0xFF)
temporarily deployed
to
staging
August 14, 2026 06:45 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Adam Lastowka (Rachmanin0xFF)
force-pushed
the
Rachmanin0xFF/661-pyspark-registry-zipimport
branch
from
August 14, 2026 06:54
5351470 to
fe83cdb
Compare
Adam Lastowka (Rachmanin0xFF)
temporarily deployed
to
staging
August 14, 2026 06:55 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Adam Lastowka (Rachmanin0xFF)
temporarily deployed
to
staging
August 14, 2026 07:22 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
This was referenced Aug 17, 2026
Collaborator
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 <adamlastowka@gmail.com>
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 <adamlastowka@gmail.com>
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 <adamlastowka@gmail.com>
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 <adamlastowka@gmail.com>
Victor Schappert (vcschapp)
force-pushed
the
Rachmanin0xFF/661-pyspark-registry-zipimport
branch
from
August 21, 2026 23:00
c9c2f84 to
a4246c5
Compare
Victor Schappert (vcschapp)
requested changes
Aug 21, 2026
Victor Schappert (vcschapp)
left a comment
Victor Schappert (vcschapp)
left a comment
Collaborator
There was a problem hiding this comment.
Gave my reasons in issue #661 why I think PR #666 is our best bet, see my comment here.
Seth Fitzsimmons (@sethfitz) Adam Lastowka (@Rachmanin0xFF) If you disagree can you align between yourselves which to put forward and close the other two? Ideally two get closed, and the remaining open one gets approved by whoever is not the author.
| @@ -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. | |||
Collaborator
There was a problem hiding this comment.
uber-nit: s/zip archive/ZIP archive/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #661
Note
This is one of two alternative fixes for #661. The other, #663, replaces filesystem discovery with a codegen-emitted index module the registry imports. Merging either one resolves the issue.
For reference, this PR has the fix that was temporarily published to CA to unblock release here.
What
overture.schema.pyspark._registry.REGISTRYcomes back empty wheneveroverture-schema-pysparkis loaded straight from a wheel onsys.pathinstead of being installed to a real directory (this is what AWS Glue does when using the--extra-py-filesoption like we do in data-platform). Every feature type then silently reports as unregistered and the task erroneously succeeds 🫠Why
_registry.pydiscovered generated modules by walking theexpressions/generated/PEP 420 namespace package withpathlib.Path(root_path).rglob("*.py"), which works... only when a real filesystem directory is present! A namespace portion backed by a zip archive has a__path__entry that points inside the zip, andpathlibcan't traverse into one.importlib.resources.files()looked like the natural fix (itsTraversableAPI is meant to be zipimport-aware), but itsMultiplexedPathimplementation raisesNotADirectoryErroron Python 3.10 the moment any namespace portion isn't a real directory.Fix
Each namespace portion is now walked directly using
zipfile.ZipFile.namelist()for portions that resolve to a path inside a zip file.Verification
sys.pathwithout extracting it (nopip install, matching Glue's--extra-py-files): all 15 generated modules now resolve; previouslylen(REGISTRY) == 0.expressions/generated/is absent entirely (make clean-pyspark): unchanged, still empty and non-crashing.