Fix DAG.cli() crashing on dags pause and unpause - #72109
Conversation
A Dag file run as a script goes through DAG.cli(), whose parser drops --dag-id and instead dispatches every subcommand with the Dag object as a second positional argument. The pause and unpause handlers never accepted it, so Dag authors got a TypeError instead of the command. The handlers are only reachable this way from a Dag file, which is why the regular `airflow dags pause` path has always worked and no test covered the difference.
ColtenOuO
left a comment
There was a problem hiding this comment.
Great work!
However, once the issue addressed in this PR is resolved, it seems to introduce another problem.
The Dag-scoped parser drops --dag-id but keeps --treat-dag-id-as-regex, so the flag survives into a context where the user has no pattern left to supply. The Dag's own id was then read back as a pattern, and dag_ids may contain dots, so an unanchored match could pause unrelated Dags. Normalising the flag next to the dag_id it guards covers both of its readers, including the confirmation prompt, rather than guarding each reader in turn and leaving the next one to be found later.
ColtenOuO
left a comment
There was a problem hiding this comment.
Thanks, Looks great!
Let's wait for maintainer take a more look.
|
Just applied change, i will merge when ci green :) |
|
Hi maintainer, this PR was merged without a milestone set.
|
Backport successfully created: v3-3-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
…72109) * Fix DAG.cli() crashing on dags pause and unpause A Dag file run as a script goes through DAG.cli(), whose parser drops --dag-id and instead dispatches every subcommand with the Dag object as a second positional argument. The pause and unpause handlers never accepted it, so Dag authors got a TypeError instead of the command. The handlers are only reachable this way from a Dag file, which is why the regular `airflow dags pause` path has always worked and no test covered the difference. * Ignore --treat-dag-id-as-regex when DAG.cli() supplies the Dag The Dag-scoped parser drops --dag-id but keeps --treat-dag-id-as-regex, so the flag survives into a context where the user has no pattern left to supply. The Dag's own id was then read back as a pattern, and dag_ids may contain dots, so an unanchored match could pause unrelated Dags. Normalising the flag next to the dag_id it guards covers both of its readers, including the confirmation prompt, rather than guarding each reader in turn and leaving the next one to be found later. * Update airflow-core/tests/unit/cli/commands/test_dag_command.py --------- (cherry picked from commit bcc430f) Co-authored-by: Y-C <easoneason0905@gmail.com> Co-authored-by: Eason09053360 <185830721+Eason09053360@users.noreply.github.com> Co-authored-by: Henry Chen <henryhenry0512@gmail.com>
…#72565) * Fix DAG.cli() crashing on dags pause and unpause A Dag file run as a script goes through DAG.cli(), whose parser drops --dag-id and instead dispatches every subcommand with the Dag object as a second positional argument. The pause and unpause handlers never accepted it, so Dag authors got a TypeError instead of the command. The handlers are only reachable this way from a Dag file, which is why the regular `airflow dags pause` path has always worked and no test covered the difference. * Ignore --treat-dag-id-as-regex when DAG.cli() supplies the Dag The Dag-scoped parser drops --dag-id but keeps --treat-dag-id-as-regex, so the flag survives into a context where the user has no pattern left to supply. The Dag's own id was then read back as a pattern, and dag_ids may contain dots, so an unanchored match could pause unrelated Dags. Normalising the flag next to the dag_id it guards covers both of its readers, including the confirmation prompt, rather than guarding each reader in turn and leaving the next one to be found later. * Update airflow-core/tests/unit/cli/commands/test_dag_command.py --------- (cherry picked from commit bcc430f) Co-authored-by: Y-C <easoneason0905@gmail.com> Co-authored-by: Eason09053360 <185830721+Eason09053360@users.noreply.github.com> Co-authored-by: Henry Chen <henryhenry0512@gmail.com>
DAG.cli()dispatches every subcommand with the Dag object as a secondpositional argument, because its parser drops
--dag-id. Thedags pauseand
dags unpausehandlers never accepted it, so running a Dag file as ascript raised
TypeError: dag_pause() takes 1 positional argument but 2 were giveninstead of the command.The regular
airflow dags pause --dag-id foopath has always worked, whichis why this went unnoticed — the handlers are only reachable with two
arguments from a Dag file.
Only
pauseandunpausewere affected.DAG_CLI_DICTresolves to exactlysix subcommands —
dags list-runs/pause/unpause/testandtasks list/test—and the other four already accepted the Dag argument.
The filter at
cli_config.py:2362also listsbackfillandrun, but neithername exists in
DAGS_COMMANDS/TASKS_COMMANDSany more, so they match nothing.Harmless today, and left alone here to keep this diff focused — but it is the
same fragility that hid this bug: two layers wired together by string matching,
with nothing enforcing that the names or the call signatures line up.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines