Remove `subdir` command from task commands #47837

ambika-garg · 2025-03-17T00:49:15Z

related: #45647

ambika-garg · 2025-03-17T14:21:02Z

Hey @jedcunningham, should we remove the get_dags function in task_clear, since the task_clear command only accepts a single dag_id?

jedcunningham · 2025-03-17T14:26:27Z

airflow/cli/commands/remote_commands/task_command.py

    to have succeeded, but found 1 non-success(es).
    """
-    dag = get_dag(args.subdir, args.dag_id)
+    dag = _parse_and_get_dag(args.dag_id)


This should just use serialized dag from the db, not reparse it on the fly like this.

jedcunningham · 2025-03-17T14:26:42Z

airflow/cli/commands/remote_commands/task_command.py

    success
    """
-    dag = get_dag(args.subdir, args.dag_id)
+    dag = _parse_and_get_dag(args.dag_id)


jedcunningham · 2025-03-17T14:26:55Z

airflow/cli/commands/remote_commands/task_command.py

 def task_list(args, dag: DAG | None = None) -> None:
    """List the tasks within a DAG at the command line."""
-    dag = dag or get_dag(args.subdir, args.dag_id)
+    dag = dag or _parse_and_get_dag(args.dag_id)


jedcunningham · 2025-03-17T14:27:52Z

airflow/cli/commands/remote_commands/task_command.py

 @providers_configuration_loaded
 def task_render(args, dag: DAG | None = None) -> None:
    """Render and displays templated fields for a given task."""
+    dag = dag or _parse_and_get_dag(args.dag_id)


jedcunningham · 2025-03-17T14:31:18Z

should we remove the get_dags function in task_clear, since the task_clear command only accepts a single dag_id?

Not sure without digging in more. Might be some leftover cruft from subdags possibly?

ambika-garg · 2025-03-20T04:49:51Z

should we remove the get_dags function in task_clear, since the task_clear command only accepts a single dag_id?

Not sure without digging in more. Might be some leftover cruft from subdags possibly?

I was referring to this function.
https://github.com/apache/airflow/blob/main/airflow/cli/commands/remote_commands/task_command.py#L446

ambika-garg · 2025-03-21T01:30:02Z

airflow/cli/commands/remote_commands/task_command.py

+    if args.dag_id and not args.dag_regex and not args.task_regex:
        dags = [get_dag_by_file_location(args.dag_id)]
    else:
        # todo clear command only accepts a single dag_id. no reason for get_dags with 's' except regex?


I think this function should be maintained because if the user wants to use regex, dag_id is treated as a regex.

So, I made the changes accordingly.

ambika-garg · 2025-03-24T13:40:14Z

Hey @jedcunningham, can you please take a look and help me understand why the test case is failing?

jason810496

Roughly tracing all the way down, it seems that the task_to_execute we pass to ExecutionCallableRunner

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/taskinstance.py#L610

is a BaseOperator instead of a PythonOperator (or another Python-based operator inheriting from BaseOperator). Compared to the example_python_operator DAG

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/example_dags/example_python_operator.py

this results in a NotImplementedError being raised from BaseOperator.execute

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/baseoperator.py#L441-L449

jason810496 · 2025-03-24T16:19:12Z

airflow-core/tests/unit/cli/commands/remote_commands/test_task_command.py

@@ -1,4 +1,4 @@
-#
+# No code was selected, so we can't improve anything.#


Nit: Seems like your colleague has left here xD

ambika-garg · 2025-03-25T13:26:34Z

Roughly tracing all the way down, it seems that the task_to_execute we pass to ExecutionCallableRunner

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/taskinstance.py#L610

is a BaseOperator instead of a PythonOperator (or another Python-based operator inheriting from BaseOperator). Compared to the example_python_operator DAG

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/example_dags/example_python_operator.py

this results in a NotImplementedError being raised from BaseOperator.execute

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/baseoperator.py#L441-L449

Thanks for the amazing insights, @jason810496! I’m wondering why these are failing since nothing related to them is being changed in the PR.

jason810496 · 2025-03-26T01:17:45Z

Thanks for the amazing insights, @jason810496! I’m wondering why these are failing since nothing related to them is being changed in the PR.

I'm not sure at the moment, so we need to dig along the path I described to find out.

potiuk · 2025-03-31T00:05:46Z

Always rebase and solve conflicts first.

airflow-core/tests/unit/cli/commands/test_task_command.py

jason810496

I think #49317 will include the scope of this PR right ?

cc @ephraimbuddy

ephraimbuddy · 2025-04-16T07:59:47Z

I think #49317 will include the scope of this PR right ?

cc @ephraimbuddy

Yeah.

boring-cyborg bot added the area:CLI label Mar 17, 2025

jedcunningham reviewed Mar 17, 2025

View reviewed changes

ambika-garg commented Mar 21, 2025

View reviewed changes

ambika-garg force-pushed the remove-subarg-task-command branch from 0a9c459 to 6ac6d36 Compare March 23, 2025 18:29

jason810496 reviewed Mar 24, 2025

View reviewed changes

ambika-garg force-pushed the remove-subarg-task-command branch from 7833efd to 0e644d7 Compare March 26, 2025 14:12

ambika-garg added 7 commits April 4, 2025 18:01

Remove subdir command from task commands

e07f481

Fix the tasks functions to read dag from serialized table

a543777

Remove subdir from the task_clear function

5303157

Fix test test_get_dags

62f98d3

Fix import error

1b9ec81

Remove unnecessary fixtures

3bc68b0

Fix task_test to avoid reparsing the dag

bb1fb08

ambika-garg force-pushed the remove-subarg-task-command branch from 0e644d7 to bb1fb08 Compare April 6, 2025 17:46

ambika-garg and others added 2 commits April 7, 2025 11:01

Merge branch 'main' into remove-subarg-task-command

706caa5

Merge branch 'main' into remove-subarg-task-command

3b6e268

jedcunningham reviewed Apr 15, 2025

View reviewed changes

airflow-core/tests/unit/cli/commands/test_task_command.py Outdated Show resolved Hide resolved

jedcunningham and others added 2 commits April 15, 2025 08:44

Update airflow-core/tests/unit/cli/commands/test_task_command.py

23bdf97

Merge branch 'main' into remove-subarg-task-command

a335494

ambika-garg marked this pull request as ready for review April 15, 2025 22:22

jason810496 reviewed Apr 16, 2025

View reviewed changes

ambika-garg closed this Apr 18, 2025

		@@ -1,4 +1,4 @@
		#
		# No code was selected, so we can't improve anything.#

Remove subdir command from task commands #47837

Remove subdir command from task commands #47837

Uh oh!

Conversation

ambika-garg commented Mar 17, 2025

Uh oh!

ambika-garg commented Mar 17, 2025

Uh oh!

jedcunningham Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jedcunningham Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jedcunningham Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jedcunningham Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jedcunningham commented Mar 17, 2025

Uh oh!

ambika-garg commented Mar 20, 2025

Uh oh!

ambika-garg Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

ambika-garg Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

ambika-garg commented Mar 24, 2025

Uh oh!

jason810496 left a comment

Choose a reason for hiding this comment

Uh oh!

jason810496 Mar 24, 2025 • 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!

ambika-garg commented Mar 25, 2025

Uh oh!

jason810496 commented Mar 26, 2025

Uh oh!

potiuk commented Mar 31, 2025

Uh oh!

Uh oh!

jason810496 left a comment

Choose a reason for hiding this comment

Uh oh!

ephraimbuddy commented Apr 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Remove `subdir` command from task commands #47837

Remove `subdir` command from task commands #47837

jason810496 Mar 24, 2025 •

edited

Loading