Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg closed pull request #47837: Remove `subdir` command from task commands URL: https://github.com/apache/airflow/pull/47837 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ephraimbuddy commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2808731407 > I think #49317 will include the scope of this PR right ? > > cc @ephraimbuddy Yeah. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
jedcunningham commented on code in PR #47837: URL: https://github.com/apache/airflow/pull/47837#discussion_r2044804970 ## airflow-core/tests/unit/cli/commands/test_task_command.py: ## @@ -1,4 +1,4 @@ -# +# No code was selected, so we can't improve anything.# Review Comment: ```suggestion # ``` -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
jason810496 commented on code in PR #47837: URL: https://github.com/apache/airflow/pull/47837#discussion_r2010509737 ## 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.# Review Comment: Nit: Seems like your colleague has left here xD -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
potiuk commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2764818323 Always rebase and solve conflicts first. https://github.com/user-attachments/assets/1d8a462d-b170-4280-85ec-989bf25568f3"; /> -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
jason810496 commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2752966230 > 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. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2751260019 > 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. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
jason810496 commented on code in PR #47837: URL: https://github.com/apache/airflow/pull/47837#discussion_r2010509737 ## 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.# Review Comment: Nit: Seems like your colleague has left here. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2748172550 Hey @jedcunningham, can you please take a look and help me understand why the test case is failing? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg commented on code in PR #47837: URL: https://github.com/apache/airflow/pull/47837#discussion_r2006703374 ## airflow/cli/commands/remote_commands/task_command.py: ## @@ -447,12 +456,11 @@ def task_clear(args) -> None: """Clear all task instances or only those matched by regex for a DAG(s).""" logging.basicConfig(level=settings.LOGGING_LEVEL, format=settings.SIMPLE_LOG_FORMAT) -if args.dag_id and not args.subdir and not args.dag_regex and not args.task_regex: +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? Review Comment: So, I made the changes accordingly. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg commented on code in PR #47837: URL: https://github.com/apache/airflow/pull/47837#discussion_r2006702826 ## airflow/cli/commands/remote_commands/task_command.py: ## @@ -447,12 +456,11 @@ def task_clear(args) -> None: """Clear all task instances or only those matched by regex for a DAG(s).""" logging.basicConfig(level=settings.LOGGING_LEVEL, format=settings.SIMPLE_LOG_FORMAT) -if args.dag_id and not args.subdir and not args.dag_regex and not args.task_regex: +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? Review Comment: I think this function should be maintained because if the user wants to use regex, `dag_id` is treated as a regex. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2739143449 > > 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 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
jedcunningham commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2729736730 > 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? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
jedcunningham commented on code in PR #47837: URL: https://github.com/apache/airflow/pull/47837#discussion_r1998860497 ## airflow/cli/commands/remote_commands/task_command.py: ## @@ -231,7 +231,13 @@ def task_failed_deps(args) -> None: Trigger Rule: Task's trigger rule 'all_success' requires all upstream tasks 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) Review Comment: This should just use serialized dag from the db, not reparse it on the fly like this. ## airflow/cli/commands/remote_commands/task_command.py: ## @@ -255,7 +261,13 @@ def task_state(args) -> None: >>> airflow tasks state tutorial sleep 2015-01-01 success """ -dag = get_dag(args.subdir, args.dag_id) +dag = _parse_and_get_dag(args.dag_id) Review Comment: Same here. ## airflow/cli/commands/remote_commands/task_command.py: ## @@ -266,7 +278,13 @@ def task_state(args) -> None: @providers_configuration_loaded 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) Review Comment: And here. ## airflow/cli/commands/remote_commands/task_command.py: ## @@ -421,8 +444,13 @@ def task_test(args, dag: DAG | None = None, session: Session = NEW_SESSION) -> N @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) Review Comment: And here. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `subdir` command from task commands [airflow]
ambika-garg commented on PR #47837: URL: https://github.com/apache/airflow/pull/47837#issuecomment-2729699568 Hey @jedcunningham, should we remove the `get_dags` function in `task_clear`, since the `task_clear` command only accepts a single `dag_id`? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
