You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bingqin2 (via GitHub)" <gi...@apache.org> on 2026/09/10 22:52:09 UTC
[PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
bingqin2 opened a new pull request, #72906:
URL: https://github.com/apache/airflow/pull/72906
`DAG.test()` re-walks the whole bundle that owns the tested Dag on every call: #66205 added that so Dags triggered through `TriggerDagRunOperator` exist in the metadata DB even when the parent was serialized earlier (#64884). The cost is that debugging one Dag parses and re-syncs every Dag file in the bundle each time, which is what #72513 reports.
This keeps the guarantee from #66205 but narrows the work in the common case. Within the owning bundle, `DAG.test()` now parses only the file that defines the tested Dag plus the files of the Dags it triggers, when those files are known. It still walks the whole bundle when they are not:
- the tested Dag is not defined inside the bundle
- a trigger target is not resolvable before runtime (a templated `trigger_dag_id`, an XComArg, or a mapped operator expanding over it)
- a target has never been parsed, so the metadata DB does not know which file defines it (the #64884 case)
Trigger targets are found through the `trigger_dag_id` template field rather than the operator class, so the Task SDK does not import the standard provider. The bundle-selection logic (owning bundle first, all bundles as fallback) and the "no parsing context" rule from #66205 are unchanged.
**Changes**
- `task-sdk/src/airflow/sdk/definitions/dag.py`: build the `BundleDagBag` without collecting, parse the selected files with `collect_dags(dag_folder=<file>)`, fall back to the full walk when the selection is not possible or does not yield the tested Dag; two module-level helpers, `_static_trigger_targets` and `_dag_test_files_to_sync`
- `airflow-core/tests/unit/models/test_dag.py`: `test_dag_test_parses_only_needed_files_once_targets_are_known` (second call parses one file and leaves an unrelated sibling untouched; fails on main) and `test_dag_test_walks_bundle_when_trigger_target_is_templated`
**Testing**
- `airflow-core`: `tests/unit/models/test_dag.py` (all 211 tests)
- `task-sdk`: `tests/task_sdk/definitions/test_dag.py`
- mypy on the changed module, prek hooks on the changed files
closes: #72513
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Claude Fable 5.1) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions). I reviewed and understand all changes; the tests were run locally as listed above.
---
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
Posted by "bingqin2 (via GitHub)" <gi...@apache.org>.
bingqin2 commented on PR #72906:
URL: https://github.com/apache/airflow/pull/72906#issuecomment-6240001001
Rebased onto current main (dba55d8728) as a single commit. The conflicts were only in the test files, next to tests added on main since; the `DAG.test()` change is the same. `airflow-core` `tests/unit/models/test_dag.py`: 236 passed. `task-sdk` `tests/task_sdk/definitions/test_dag.py`: 127 passed, including the `TestDagTestFilesToSync` tests requested in the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
Posted by "aaron-y-chen (via GitHub)" <gi...@apache.org>.
aaron-y-chen commented on code in PR #72906:
URL: https://github.com/apache/airflow/pull/72906#discussion_r4351479954
##########
task-sdk/src/airflow/sdk/definitions/dag.py:
##########
@@ -1378,7 +1384,14 @@ def test(
dag_folder=bundle.path,
bundle_path=bundle.path,
bundle_name=bundle.name,
+ collect_dags=False,
)
+ files_to_parse = _dag_test_files_to_sync(self, bundle, session=session)
+ if files_to_parse is not None:
+ for file_path in files_to_parse:
+ dagbag.collect_dags(dag_folder=file_path)
+ if files_to_parse is None or self.dag_id not in dagbag.dags:
Review Comment:
After an initial sync, if `child` moves from `old.py` to `new.py` while `old.py` remains for another Dag, `parent.test()` appears to trigger the stale serialized child. The recorded path still exists, so only `parent.py` and `old.py` are parsed. Since the parent is collected, this condition skips bundle discovery even though `child` is absent from the bag. The updated child in `new.py` is never synced, and the trigger uses its previous definition from the metadata DB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
Posted by "bingqin2 (via GitHub)" <gi...@apache.org>.
bingqin2 commented on PR #72906:
URL: https://github.com/apache/airflow/pull/72906#issuecomment-5626503511
cc @kaxil since this builds on #66205
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
Posted by "bingqin2 (via GitHub)" <gi...@apache.org>.
bingqin2 commented on PR #72906:
URL: https://github.com/apache/airflow/pull/72906#issuecomment-5915146425
@jroachgolf84 the Task SDK tests you asked for are in: `TestDagTestFilesToSync` in `task-sdk/tests/task_sdk/definitions/test_dag.py` covers both helpers without the database (13 tests). Could you take another look when you have time? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
Posted by "bingqin2 (via GitHub)" <gi...@apache.org>.
bingqin2 commented on PR #72906:
URL: https://github.com/apache/airflow/pull/72906#issuecomment-5651305546
A quick measurement of the repeated call, since that is what #72513 is about. Setup: the `configure_testing_dag_bundle` fixture with the parent/child Dags from `test_dag_test_with_trigger.py` plus N generated one-task sibling Dags; timing `parent.test()` after the first call (the first call still walks the bundle on both sides, 4.2 s vs 4.6 s, within noise).
| siblings in the bundle | main (6a6a24f2) | this PR |
|---|---|---|
| 50 (the unit-test `dags` folder) | 0.35–0.48 s | 0.08–0.10 s |
| 300 | 0.58–0.81 s | 0.10–0.11 s |
On main every later call re-imports each sibling file and runs it through the sync again (the serialized hash is unchanged, so the DB write itself is skipped, which is why one-task siblings stay cheap). On this branch the call parses one file, and the cost no longer grows with the bundle. Real Dag files with heavier imports or many tasks widen the gap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Parse only the tested Dag and its trigger targets in DAG.test() [airflow]
Posted by "bingqin2 (via GitHub)" <gi...@apache.org>.
bingqin2 commented on PR #72906:
URL: https://github.com/apache/airflow/pull/72906#issuecomment-5666373636
Thanks, added. `task-sdk/tests/task_sdk/definitions/test_dag.py` now covers the two helpers directly, without the database (`DagModel.get_current` is patched, since Task SDK tests must not use one):
- `_static_trigger_targets`: literal ids are collected and the Dag's own id ignored, a Dag without trigger tasks yields an empty set, and a templated value, an XComArg, or a mapped operator expanding over `trigger_dag_id` make the result `None` (a mapped operator with a fixed `trigger_dag_id` in `partial()` is still static)
- `_dag_test_files_to_sync`: the Dag's own file alone, target files resolved through `relative_fileloc` or the absolute `fileloc` with targets from another bundle skipped, and every whole-bundle fallback: the Dag defined outside the bundle, a non-static target, a target never parsed, a target without a recorded location, and a target file missing or resolving outside the bundle
The operator used is a small `BaseOperator` subclass with a `trigger_dag_id` template field, so the suite does not depend on the standard provider. The airflow-core tests stay as the end-to-end check of `DAG.test()` itself, which needs the bundle and the metadata DB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org