Add task instance management commands to airflowctl CLI#67947
Add task instance management commands to airflowctl CLI#67947Suraj-kumar00 wants to merge 19 commits into
Conversation
bfb913a to
193ed6a
Compare
|
Hello @potiuk, Also could you help me get this that do i have to do the testing manually on local? This is taking so much time locally!!! |
bugraoz93
left a comment
There was a problem hiding this comment.
There are some foundational things needs changing, thanks for the PR!
0b76de6 to
400f426
Compare
4da1910 to
a584bd1
Compare
|
@Suraj-kumar00 A few things need addressing before review — see our Pull Request quality criteria.
No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
|
Hi @potiuk, I'll make the changes tonight! |
| if ( | ||
| expanded_parameter in args_dict.keys() | ||
| and args_dict[expanded_parameter] is not None | ||
| ): | ||
| val = args_dict[expanded_parameter] |
There was a problem hiding this comment.
If I understand correctly, the None filtering is not strictly required for fields where null and “not provided” are equivalent. More importantly, this new path does not protect boolean fields, because the generated CLI currently defaults bool arguments to False. For ClearTaskInstancesBody, some API defaults are True or None (dry_run, only_failed, reset_dag_runs, run_on_latest_version), so airflowctl tasks clear <dag_id> may send explicit False values and override the API defaults.
There was a problem hiding this comment.
Yeah, you're right on both counts.
Fixed by changing the default from False to None for bool fields in _create_arg_for_non_primitive_type, so unset flags are now omitted from the request body and the API defaults apply. This also brings the path in sync with the primitive arg path, which already defaulted to None.
Added a regression test (test_command_factory_body_bool_field_defaults_to_none) using ClearTaskInstancesBody to lock it in.
| ansi_escape = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") | ||
| out_str = ansi_escape.sub("", captured.out).replace("\n", "") | ||
| # Since rich wraps long lines, newlines are removed, so we assert the combined text. | ||
| assert out_str == f"Exported {len(exported_data)} pool(s) to {export_file}" |
There was a problem hiding this comment.
Could I ask why we need to change this?
There was a problem hiding this comment.
tbh, this was unintentional scope creep from local debugging of rich-rendered output. Reverted in the latest push.
|
Hello @henry3260, I'm looking into this. Thanks for the review! |
bugraoz93
left a comment
There was a problem hiding this comment.
I agree with Henry's comments and could you please check CI what is failing on the integration tests and static checks
…tanceOperations - Remove bare re-raise try/except from get, clear, and update methods - Fix ruff E302: add missing blank line before TaskInstanceOperations class - Fix ruff F841: remove unused variable in pool command export test - Run ruff format on operations.py, cli_config.py, test_operations.py
…task_ids validation
…onal args Required path params (dag_id, dag_run_id, task_id, key, value) are generated as positional CLI arguments by CommandFactory, not optional --flags. Also fix ruff-format: remove extra blank lines before TaskInstanceOperations and TestTaskInstanceOperations class definitions.
81a9ef1 to
f602199
Compare
|
Hi @henry3260 @potiuk, I have made the changes as per the feedback, could you please review? |
Adds task instance management support to the airflowctl CLI, enabling users to get, list, clear, and update task instances directly from the terminal.
Closes: #61547
Related: #66173
Changes
airflow-ctl/src/airflowctl/api/operations.pyTasksOperationsclass (matching Airflow core CLI naming) withget,list,clear, andupdatemethodsPluginsOperationsclass that was lost during merge with main (Added plugins command to airflowctl #64935)airflow-ctl/src/airflowctl/api/client.pytasksproperty toClientfor auto-generated CLI command wiringpluginsproperty that was lost during merge with mainairflow-ctl/src/airflowctl/ctl/cli_config.py"clear"tooutput_command_listfor correct CLI output handlingairflow-ctl/src/airflowctl/ctl/help_texts.yamltasks:section with help text for all four subcommandsscripts/in_container/run_capture_airflowctl_help.py"tasks"to the commands list for SVG/help image generationairflow-ctl/tests/airflow_ctl/api/test_operations.pyTestTasksOperationswith 7 unit tests covering get, list, clear, and updateTestPluginsOperationstests lost during mergeairflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.pytaskscommands using positional argumentsCLI Commands