fix(core): never write unparseable entity.py from fetch_schema - #167
Open
LukasGold wants to merge 2 commits into
Open
fix(core): never write unparseable entity.py from fetch_schema#167LukasGold wants to merge 2 commits into
LukasGold wants to merge 2 commits into
Conversation
- strip datamodel-code-generator's repr'd `object()` default sentinel - widen oold's regex so a sentinel wrapped in parse_obj() also repairs - validate generated content with ast.parse before overwriting the file - keep the previous valid entity.py in place when generation is broken - log black/isort failures instead of swallowing them silently
Contributor
Release previewMerging this PR would release v2.3.2 (current: Changelog preview (truncated)## v2.3.2 (2026-09-03)
### Bug Fixes
- **core**: Never write unparseable entity.py from fetch_schema
([`2f68c91`](https://github.com/OpenSemanticLab/osw-python/commit/2f68c91f4663d0f05a0ebee3390a9287140547a3))
- **core**: Roll back entity.py when the generated model cannot import
([`a409dac`](https://github.com/OpenSemanticLab/osw-python/commit/a409dacba600190e9f49c327cfcfffd892280233))
Preview via python-semantic-release and conventional commits. |
- ast.parse only proves syntax, not that the module imports - restore the previous file content if importlib.reload raises - reload again after the rollback so the module works in memory - re-raise the original import error instead of masking it
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 #125
OSW._fetch_schemacould write a syntactically invalidsrc/osw/model/entity.pyand thenimportlib.reload()it, raisingSyntaxErrorand breaking every laterimport osw.corein that process. Because the file is written unconditionally, the corruption persisted on disk even when the call was notfinaland no reload surfaced it.Root cause
datamodel-code-generatormarks "no default" with a bareUNDEFINED = object()sentinel guarded byischecks.oold.utils.json_tools.merge_deepdeep-copies schema dicts duringallOfcomposition, which clones the sentinel into a look-alike object and defeats the identity guard, so the generator reprs it into source:oold already works around this in
Generator.generate(), but osw never benefits from it:_fetch_schemacallsdatamodel_code_generator.generate()directly instead of oold's wrapper, and reimplements its own post-processing without the sentinel cleanup.Changes
Repair the known bad output
remove_unserializable_default_sentinels()strips the repr'd sentinel, called alongside the existing post-processing regexes so it covers every path reaching the write.)after the address, which is the paren belonging toparse_obj(, so on the shape reported in this issue it producesField(default=None), options={...})and still does not parse. Matching the factory expression itself repairs both the bare and the call-wrapped shape.Never leave a broken entity.py behind
ensure_valid_python_source()runsast.parsebefore the file is opened for writing. On failure it logs and raises with the path, line number and offending source line, so the previous validentity.pyis left in place.ast.parseonly proves syntax, soreload_module_or_restore()covers the second half: if the generated model imports badly (undefined name, error raised while a class body executes), the previous file content is written back, the module is reloaded again so it also works in memory, and the original error is re-raised.Stop hiding the warning signs
black/isortfailure is logged at warning level instead ofexcept Exception: pass. Behaviour is unchanged otherwise: formatting stays optional.Notes
entity.py.finalcall still writes without an import check, because intermediate content is not expected to be importable yet, which is why the existing code defers the reload tofinal. Such a write is still syntax-checked._fetch_schemawriting into the installed package tree at all, which is fetch_schema rewrites src/osw/model/entity.py as a side effect of ordinary read/write calls #141.Tests
tests/test_fetch_schema_write_safety.py, 9 tests, fully offline, never invoking the realfetch_schema:parse_obj-wrapped sentinel from the issue both become valid Python, with trailingFieldkwargs preserveddefault_factory=lambda: uuid4()is untouched200 passed, 1 skippedoffline;ruff checkclean.