Keep ZIP-archived Dags active when dag_discovery_safe_mode is False by Vamsi-klu · Pull Request #68518 · apache/airflow

Vamsi-klu · 2026-06-13T22:03:32Z

When [core] dag_discovery_safe_mode is False, Dags defined in ZIP archives whose source does not contain the airflow/dag keywords were parsed and activated by the Dag file processor but then immediately deactivated on the next scan. _get_observed_filelocs expanded ZIP members with a hardcoded safe_mode=True, so keyword-less members were absent from the observed set and deactivate_deleted_dags marked them stale.

Changes:

Resolve dag_discovery_safe_mode once on DagFileProcessorManager and use it for both file discovery (_find_files_in_bundle) and the observed-filelocs scan (_get_observed_filelocs), fixing the hardcoded True.
Log the effective dag_discovery_safe_mode per bundle. In separate-process deployments (Helm/OpenShift) the flag must be set on the dag-processor and the component restarted; otherwise Dags appear after airflow dags reserialize and disappear on the next scan (the plain-.py symptom in the issue). The log makes that misconfiguration diagnosable.
Clarify the config.yml / docs guidance accordingly.

Tested: new parametrized regression test test_get_observed_filelocs_respects_dag_discovery_safe_mode (keyword-less ZIP member is observed when safe_mode is False, filtered when True).

Was generative AI tooling used to co-author this PR?

Yes — Claude Code (Opus 4.8)

Generated-by: Claude Code (Opus 4.8) following the guidelines

Important

🛠️ Maintainer triage note for @Vamsi-klu · by @potiuk · 2026-07-02 17:46 UTC

Helpful heads-up from the maintainers — please address before this PR can be reviewed (see the Pull Request quality criteria):

This PR has merge conflicts with main. Please rebase onto the latest main and resolve them.

The ball is in your court — you've been assigned to this PR. Fix the above, then mark it Ready for review.

_{Automated triage — may be imperfect; a maintainer takes the next look.}

Vamsi-klu · 2026-07-03T01:47:23Z

Rebased this PR onto current upstream/main, resolved the Dag processor manager conflict, and marked it ready for review.

The conflict resolution keeps upstream lazy execution API initialization and applies dag_discovery_safe_mode consistently to both bundle file discovery and ZIP-member observation.

Validation run:

uv run ruff format airflow-core/src/airflow/dag_processing/manager.py airflow-core/tests/unit/dag_processing/test_manager.py
uv run ruff check --fix airflow-core/src/airflow/dag_processing/manager.py airflow-core/tests/unit/dag_processing/test_manager.py
uv run --project airflow-core pytest airflow-core/tests/unit/dag_processing/test_manager.py::TestDagFileProcessorManager::test_get_observed_filelocs_respects_dag_discovery_safe_mode -xvs
prek run ruff --from-ref upstream/main
prek run ruff-format --from-ref upstream/main

PR is now mergeable; required review/checks remain.

Drafted-by: Codex (GPT-5)

vatsrahul1001

Nice catch fixing both spots here. One gap: there's no regression test for the _find_files_in_bundle fix, only for _get_observed_filelocs. Could you add a parametrized test mirroring the existing one, asserting a keyword-less .py file is included/excluded based on dag_discovery_safe_mode? Otherwise a future change could silently reintroduce that half of the bug.

The Dag file processor parsed and activated keyword-less Dags inside ZIP archives but then immediately deactivated them, because the scan that decides which files still exist ignored dag_discovery_safe_mode and always applied the airflow/dag keyword heuristic. Resolve the setting once and use it consistently for discovery and the deactivation scan, and log the effective value so a misconfigured processor is diagnosable. closes: apache#66104

The file-discovery and observed-file paths must both honor safe mode so wrapped Dag files are not silently skipped by a future regression.

Vamsi-klu · 2026-07-31T06:48:43Z

Added the parameterized _find_files_in_bundle safe-mode regression test in 48af3f7.
Covers keyword-present and keyword-less plain .py discovery with safe_mode on/off (mirrors the existing _get_observed_filelocs coverage for ZIP members). Re-requesting review.

Vamsi-klu

@vatsrahul1001 Addressed your feedback: added parametrized test_find_files_in_bundle_respects_dag_discovery_safe_mode in 48af3f7 (safe_mode on/off for keyword-less .py discovery). Ready for another look.

Vamsi-klu · 2026-08-01T04:37:10Z

Good catch, thanks. That test is already on the current head (48af3f7): test_find_files_in_bundle_respects_dag_discovery_safe_mode in test_manager.py, sitting right next to the observed-filelocs one. Same shape: a keyword-less .py file, parametrized over both safe_mode values. All 4 cases pass for me locally. PTAL when you get a chance.

Drafted-by: Claude Code (Fable 5); reviewed by @Vamsi-klu before posting

vatsrahul1001 · 2026-08-03T05:56:32Z

LGTM!, can be merged after code owner review @ephraimbuddy @jedcunningham

ephraimbuddy

LGTM

The tests bypass the config wiring. Can you make it use it?

ephraimbuddy · 2026-08-03T13:58:13Z

+    """Resolved once per process so file discovery and the deactivation scan use the same value.
+
+    When ``False`` the keyword heuristic is bypassed and every Python file is scanned -- this must
+    apply consistently to discovery (:meth:`_find_files_in_bundle`) and to the set of observed
+    filelocs (:meth:`_get_observed_filelocs`); otherwise freshly-parsed keyword-less Dags are
+    deactivated right after being parsed (issue #66104)."""


Suggested change

"""Resolved once per process so file discovery and the deactivation scan use the same value.

When ``False`` the keyword heuristic is bypassed and every Python file is scanned -- this must

apply consistently to discovery (:meth:`_find_files_in_bundle`) and to the set of observed

filelocs (:meth:`_get_observed_filelocs`); otherwise freshly-parsed keyword-less Dags are

deactivated right after being parsed (issue #66104)."""

"""Resolved once per process so file discovery and the deactivation scan use the same value."""

This should just be one line comment

Done in a45c514, collapsed to the suggested one-line docstring.

Drafted-by: Claude Code (Fable 5); reviewed by @Vamsi-klu before posting

Good call, done in a45c514. It's just the one-liner you suggested now.

Drafted-by: Claude Code (Fable 5); reviewed by @Vamsi-klu before posting

ephraimbuddy · 2026-08-03T14:04:33Z

+    ``with_keywords.py`` contains the ``airflow``/``dag`` strings the safe-mode heuristic looks
+    for. ``no_keywords.py`` mimics a custom wrapper whose source contains neither ``airflow`` nor
+    ``dag``/``asset`` -- exactly the case ``dag_discovery_safe_mode=False`` exists to support
+    (issue #66104).


Issue numbers shouldn't be in codes except if it's a todo

Dropped it in a45c514. While I was at it I noticed one more #66104 reference in another test docstring in the same file, so I cleaned that one up too. None left now.

Drafted-by: Claude Code (Fable 5); reviewed by @Vamsi-klu before posting

ephraimbuddy · 2026-08-03T14:08:56Z

-                        if might_contain_dag(info.filename, True, z):
+                        # Must use the same safe_mode as discovery/parsing: with a hardcoded True a
+                        # keyword-less zip member parsed under safe_mode=False would be absent here and
+                        # then deactivated immediately after being parsed (issue #66104).


Remove issue number and make this comment really short e.g use configured discovery safe mode

Done, went with your wording in a45c514.

Drafted-by: Claude Code (Fable 5); reviewed by @Vamsi-klu before posting

Review asked for one-line comments and no issue numbers in code, since the PR link already carries that context.

boring-cyborg Bot added area:ConfigTemplates area:DAG-processing kind:documentation labels Jun 13, 2026

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from 2e35a34 to 11741f4 Compare June 13, 2026 22:03

Vamsi-klu marked this pull request as ready for review June 13, 2026 22:14

Vamsi-klu requested review from ephraimbuddy and jedcunningham as code owners June 13, 2026 22:14

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from 11741f4 to 942843f Compare June 14, 2026 02:24

eladkal added this to the Airflow 3.3.0 milestone Jun 15, 2026

potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jun 17, 2026

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from 942843f to 13d95bc Compare June 20, 2026 16:32

vatsrahul1001 modified the milestones: Airflow 3.3.0, Airflow 3.3.1 Jun 25, 2026

potiuk assigned Vamsi-klu Jul 2, 2026

potiuk marked this pull request as draft July 2, 2026 17:47

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from 13d95bc to 78350c2 Compare July 3, 2026 01:46

Vamsi-klu marked this pull request as ready for review July 3, 2026 01:47

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from 78350c2 to a9790a8 Compare July 3, 2026 01:47

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from a9790a8 to 2283633 Compare July 20, 2026 14:07

vatsrahul1001 reviewed Jul 29, 2026

View reviewed changes

Vamsi-klu added 2 commits July 30, 2026 07:44

Cover safe-mode filtering in Dag bundle discovery

48af3f7

The file-discovery and observed-file paths must both honor safe mode so wrapped Dag files are not silently skipped by a future regression.

Vamsi-klu force-pushed the fix-dag-discovery-safe-mode-zip-66104 branch from 2283633 to 48af3f7 Compare July 30, 2026 07:52

vatsrahul1001 added the backport-to-v3-3-test Backport to v3-3-test label Jul 30, 2026

Vamsi-klu commented Jul 31, 2026

View reviewed changes

Vamsi-klu requested a review from vatsrahul1001 July 31, 2026 07:03

vatsrahul1001 modified the milestones: Airflow 3.3.1, Airflow 3.3.2 Aug 3, 2026

ephraimbuddy reviewed Aug 3, 2026

View reviewed changes

Shorten review-flagged comments in Dag discovery safe mode change

a45c514

Review asked for one-line comments and no issue numbers in code, since the PR link already carries that context.

Uh oh!

Conversation

Vamsi-klu commented Jun 13, 2026 • edited by potiuk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Was generative AI tooling used to co-author this PR?

Uh oh!

Vamsi-klu commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vatsrahul1001 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vamsi-klu commented Jul 31, 2026

Uh oh!

Vamsi-klu left a comment

Choose a reason for hiding this comment

Uh oh!

Vamsi-klu commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vatsrahul1001 commented Aug 3, 2026

Uh oh!

ephraimbuddy left a comment

Choose a reason for hiding this comment

Uh oh!

ephraimbuddy Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Vamsi-klu Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Vamsi-klu Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ephraimbuddy Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Vamsi-klu Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

ephraimbuddy Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Vamsi-klu Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Vamsi-klu commented Jun 13, 2026 •

edited by potiuk

Loading

Vamsi-klu commented Jul 3, 2026 •

edited

Loading

vatsrahul1001 left a comment •

edited

Loading

Vamsi-klu commented Aug 1, 2026 •

edited

Loading