Repository navigation
fix(airflow): scope default args to DAG context - #178
Open
rudra337-lgtm wants to merge 2 commits into
Open
rudra337-lgtm wants to merge 2 commits into
rudra337-lgtm wants to merge 2 commits into
Conversation
A plain helper whose body only wires captured tasks (start >> t, chain, cross_downstream, set_upstream/set_downstream) was skipped by both the capture pass and the independent audit, so the DAG verified while the dependency was missing (databricks-solutions#150 repro 2). Both passes now follow the same rule: a module-level, parameterless, undecorated helper with wiring-only statements that is statically invoked, so Airflow's one-shot execution is reproduced statically and reconciliation compares identical edge sets. Unfollowed statements still fail closed through unclaimed statement checks.
This branch has not been deployed
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 #150.
Commit 1 — scope default args to DAG context
Airflow operators created after a
with DAG(...)block were still captured by flowx and inherited that DAG'sdefault_args, even though Airflow does not make unwired operators outside the context members of the DAG.The visitor now requires either an active DAG context or an explicit
dag=reference before registering a classic operator call. Added a regression test covering the context boundary.Commit 2 — follow statically invoked wiring-only helpers
Issue #150 repro 2: a plain helper whose body only wires already-captured tasks (
def wire(): start >> tfollowed bywire()) was skipped by both the capture pass and the independent source audit, so the DAG wasverifiedwhile the dependency edge was missing.Both passes now follow the same rule — module-level definition, no decorators, no parameters, wiring-only statements (shift chains,
chain/cross_downstream,set_upstream/set_downstream), and at least one static invocation — so Airflow's one-shot execution is reproduced statically and reconciliation compares identical edge sets on both sides. Statements inside a followed helper that cannot be resolved fail closed (unclaimed_dag_statement) instead of verifying with a missing edge. An uncalled helper stays uncaptured, matching Airflow (its wiring never runs).Verification
uv run pytest tests/unit/test_airflow_reconciliation.py -q— 32 passed, 0 failed (29 before this PR's work; +1 default-args regression test, +3 wiring-helper tests).uv run ruff checkclean on the touched files.uv run pytest tests/unit -q— 1386 passed, 2 skipped, 121 failed; all 121 failures reproduce identically on the pre-change branch (1de4f21) out of the box on this Windows machine (missingtzdatain the vendored Python 3.14 runtime, path-separator comparisons, provider JSON canonicalization, deployer summary reads) — no failures introduced, none of the related failures touched.tno longer in the DAG, nodefault_argsleak), repro 2 (edgestart -> tnow captured,verified), thewith DAGhelper variant, and the uncalled-helper Airflow-parity case all behave as expected.Remaining (issue part 3)
The module-wide "account for every operator call" check and the operator-wired-outside-context shape (currently fail-closed via
unclaimed_dag_statement) are larger follow-ups; both were left out of this PR to keep the change reviewable.