fix: import clgraph without PyYAML installed (0.0.7) - #76
Merged
Conversation
`clgraph/orchestrators/kestra.py` imported yaml at module scope, and
`clgraph/orchestrators/__init__.py` imports every backend eagerly, so a bare
`pip install clgraph` followed by `import clgraph` raised:
ModuleNotFoundError: No module named 'yaml'
PyYAML was never a declared dependency. It only ever arrived transitively in
development environments, which is why the full suite and every CI job stayed
green - all of them install ".[dev]". Bisected across published releases:
0.0.3 imports fine, 0.0.5 and 0.0.6 do not.
PyYAML is now imported at point of use via `_require_yaml()`. KestraOrchestrator
imports and constructs without it; only to_flow(), to_flow_with_triggers() and
to_flow_dict() need it, and they raise an ImportError naming the package and
the install command. Airflow, Dagster, Prefect and Mage emit code as text and
were never affected.
Also adds:
- a `clgraph[kestra]` extra, so the error message's install hint is real
- a `bare-install` CI job that installs the built wheel into a clean
environment with no extras and imports it - nothing in the pipeline would
have caught this otherwise
- regression tests that run in a subprocess with yaml made unimportable, since
the dev environment has PyYAML and the failure only reproduces without it
Bumps to 0.0.7.
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.
The bug
A clean
pip install clgraphfollowed byimport clgraphfails outright:clgraph/orchestrators/kestra.pyimportedyamlat module scope, andclgraph/orchestrators/__init__.pyimports every backend eagerly, so the failure propagates all the way up toimport clgraph.PyYAML is not a declared dependency — not in
dependencies, not in any extra. It only ever arrived transitively in development environments (via airflow and friends), which is why the full suite and every CI job stayed green: all of them install.[dev].Bisected across published releases:
import clgraphSo the currently published release is unusable out of the box. Found while verifying the 0.0.6 artifact on PyPI; it predates that release and is unrelated to its contents.
The fix
PyYAML is imported at point of use through a
_require_yaml()helper.KestraOrchestratornow imports and constructs without it — onlyto_flow(),to_flow_with_triggers()andto_flow_dict()need it, and they raise anImportErrorthat names the package and the install command:I checked every orchestrator, not just the reported one: Airflow, Dagster, Prefect and Mage emit code as text and import nothing beyond stdlib and
.base, so Kestra was the only one affected.Adding
pyyamlto coredependencieswould also have worked, but it would put a YAML parser in every install to serve one optional backend — the lazy import matches how the other four already avoid depending on their target systems.Also included
clgraph[kestra]extra, so the error message's install hint is real.bare-installCI job — builds the wheel, installs it into a clean venv with no extras, imports it, builds a pipeline, and asserts Kestra raises the actionable error. Every existing job installs.[dev], so nothing in the pipeline could have caught this class of bug. Runs on 3.10 and 3.13.tests/test_optional_orchestrator_deps.py) that run in a subprocess withyamlmade unimportable via ameta_pathblocker — the dev environment genuinely has PyYAML, so the failure only reproduces when it is absent. Includes atest_yaml_blocker_actually_blocksguard so the suite cannot pass vacuously if the blocker ever stops working.Test plan
ruff check/ruff format --checkclean.import clgraphworks, a pipeline builds (4 columns), and Kestra raises the actionableImportError.to_flowandto_flow_dictexercised).tyreports 52 errors, the same 52 asmain— no new ones.