Repository navigation
Fix ontology lazy import - #45
Merged
Merged
Conversation
On an install WITHOUT the [ontology] extra, `mdl init` and `mdl import erwin` crashed with ModuleNotFoundError: No module named 'pyoxigraph'. Scaffolding calls mdl_ontology.lock.Lock to write .mdl/lock.yaml, and importing that submodule ran mdl_ontology/__init__.py, which eagerly imported the RDF-backed providers/rdf_export chain (pyoxigraph). That broke the core-install promise: the pyoxigraph-free Lock was unreachable. mdl_ontology/__init__.py now exports every public name LAZILY (PEP 562 __getattr__), so importing a pyoxigraph-free member (Lock) no longer pulls the backend; a backend-dependent name imports it only when accessed. _ontology() in the CLI now force-resolves one backend-dependent export (serialize) inside its try/except, so a missing backend still fails there with the 'install modelith-dbt[ontology]' hint (exit 4) instead of leaking a raw ImportError from the command body. Regression tests: init + import erwin under a blocked RDF backend. CLI 0.6.9.
An erwin export carries object names with characters that are illegal in
a Windows filename (< > : " / \ | ? *) — e.g. a domain named "<root>".
The writer used the name verbatim as a YAML filename, so on Windows the
write crashed with OSError [Errno 22] Invalid argument (it silently
produced a literally-named file on macOS/Linux, which is why tests missed
it). This is the shared serializer every importer (erwin, OSI, reverse)
uses, so the crash hit any Windows user with such a name.
- _slug() now strips/collapses Windows-illegal characters and trailing
dots/spaces to underscores, and never returns empty. It was only
lowercasing + replacing spaces before.
- Every filename in write_model now routes through _slug (domains,
entities, relationships, subject-areas, physical tables previously used
the raw or partially-cleaned name).
- A collision guard disambiguates names that slug to the same file
("Order Line" vs "Order/Line" -> order_line.yaml + order_line-2.yaml)
so lossy slugging never silently overwrites one object with another.
The name itself is preserved in the model (a domain stays "<root>"); only
the on-disk filename is sanitised. No names are dropped.
CLI 0.6.10.
Our dev workspace (uv run) always has every optional dependency, so a command that reaches the optional ontology stack passes locally and crashes on a real `pip install modelith-dbt` without [ontology]. That is how both recent Windows crashes shipped: import erwin pulled pyoxigraph via the scaffold path, and a `<root>` domain produced a filesystem-illegal filename — neither reproducible in the dev env. Adds sandbox/clean-install/: a python:3.12-slim image that installs the locally built wheel CORE-ONLY (no extras) and runs the erwin import flow where a real user runs it — asserting the CLI loads, import erwin into an empty folder scaffolds without the ontology extra, the <root> domain writes to a safe filename, and the model validates. run.sh builds the wheel, builds the image, and runs it in one command; exits non-zero on failure so it is a pre-ship / CI gate. Verified: all checks pass on the current wheel; the env genuinely lacks pyoxigraph (negative control). Scoped to erwin import on a core install for now (the flows that broke); init --workspace / config / generate bars and an [ontology] pass are the natural extensions, plus a GitHub Actions job.
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.
Fix Windows/core-install crashes in erwin import, and verify in a clean Docker install
Summary
A fresh Windows
pip install modelith-dbt(core only, no[ontology]extra) crashedmdl import erwinin two different ways that our uv dev workspace could never reproduce. This PR fixes both, and adds an isolated Docker harness that exercises the flows in the install configuration users actually have, so this class of "green in dev, broken on install" bug is caught before shipping.The bugs
ModuleNotFoundError: No module named 'pyoxigraph'—mdl import erwin(andmdl init) into an empty folder scaffolds.mdl/lock.yamlviamdl_ontology.lock.Lock. Importing that submodule ranmdl_ontology/__init__.py, which eagerly imported the RDF-backed providers chain (pyoxigraph, the optional[ontology]extra). A core install doesn't have it, so scaffolding crashed — breaking the promise that the core CLI runs without the extra.OSError: [Errno 22] Invalid argument … \<root>.yaml— an erwin export carries a domain literally named<root>. The shared model writer (mdl_reverse.writer.write_model, used by every importer) used object names verbatim as filenames.<and>are illegal on Windows, soopen()crashed. macOS/Linux silently wrote a<root>.yamlfile, which is why local tests never caught it.The fixes
mdl_ontology/__init__.pynow exports every public name lazily (PEP 562__getattr__). Importing a pyoxigraph-free member (Lock) no longer pulls the backend; a backend-dependent name imports it only when accessed._ontology()in the CLI force-resolves one backend-dependent export inside its guard, so a genuinely missing backend still fails with themodelith-dbt[ontology]install hint (exit 4), not a raw traceback.mdl_reverse.writernow slugs every filename to a filesystem-safe form (strips< > : " / \ | ? *, control chars, trailing dots/spaces; never empty), and disambiguates collisions the slugging can create (Order Line+Order/Line→order_line.yaml+order_line-2.yaml). The object name is preserved in the model (a domain stays<root>); only the on-disk filename is sanitised. No names are dropped.Verification (the durable part)
New
sandbox/clean-install/Docker harness: builds the wheel, installs it core-only (no extras) inpython:3.12-slim, and runs the erwin import flow where a real user runs it — asserting the CLI loads, import into an empty folder scaffolds without the ontology extra, the<root>domain writes to a safe filename, and the model validates../sandbox/clean-install/run.shdoes it in one command and exits non-zero on failure (pre-ship / CI gate). Verified passing on the current wheel, with a negative control confirming the container genuinely lacks pyoxigraph.Tests
test_core_without_ontology.py: addedinit+import erwincases under a blocked RDF backend (the scaffold path that crashed).test_writer_filenames.py: illegal-char names → safe filenames + collision disambiguation.Notes
modelith-dbt0.6.8 → 0.6.10 (0.6.9 pyoxigraph, 0.6.10 filenames). VS Code extension unchanged at 0.3.16 — both fixes are CLI-side and the extension just shells out tomdl.modelith_dbt-0.6.10-py3-none-any.whl.