parkhojeong commented on code in PR #66202:
URL: https://github.com/apache/airflow/pull/66202#discussion_r3236167265


##########
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py:
##########
@@ -93,6 +93,8 @@ def date_param():
     "dags update example_bash_operator --no-is-paused",
     # Dag Run commands
     "dagrun list --dag-id example_bash_operator --state success --limit=1",
+    # Task instance commands - need a Dag run with completed tasks
+    'taskinstances get --dag-id=example_bash_operator 
--dag-run-id="manual__{date_param}" --task-id=runme_0',

Review Comment:
   This command fails because the auto-generated CLI is built from the 
`operations.py` signature, where required primitive parameters are exposed as 
positional arguments. So `taskinstances get` expects `dag_id dag_run_id 
task_id`, not `--dag-id`, `--dag-run-id`, and `--task-id`.
   
   - command: `uv run --project airflow-ctl airflowctl taskinstances get 
--dag-id=example --dag-run-id=manual__x --task-id=t` 
   - error message: `airflowctl taskinstances get command error: the following 
arguments are required: dag_id, dag_run_id, task_id, see help above.`
   
   



##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -268,6 +268,36 @@ def _load_help_texts_yaml() -> dict[str, dict[str, str]]:
     help="The Dag ID of the Dag to pause or unpause",
 )
 
+# Task Commands Args
+ARG_TASK_DAG_ID = Arg(
+    flags=("--dag-id",),
+    type=str,
+    dest="dag_id",
+    required=True,
+    help="The Dag ID",
+)
+ARG_DAG_RUN_ID = Arg(
+    flags=("--dag-run-id",),
+    type=str,
+    dest="dag_run_id",
+    required=True,
+    help="The Dag Run ID",
+)
+ARG_TASK_ID = Arg(
+    flags=("--task-id",),
+    type=str,
+    dest="task_id",
+    required=True,
+    help="The Task ID",
+)
+ARG_MAP_INDEX = Arg(
+    flags=("--map-index",),
+    type=int,
+    dest="map_index",
+    default=-1,
+    help="If set, query the mapped task instance with this map index (negative 
means non-mapped)",

Review Comment:
   ```suggestion
   ARG_MAP_INDEX = Arg(
       flags=("--map-index",),
       type=int,
       dest="map_index",
       help="If set, query the mapped task instance with this map index 
(negative means non-mapped)",
   ```
   
   Nit: Can we drop `default=-1` here? If omitted, argparse defaults this 
optional argument to `None`, which matches 
`TaskInstancesOperations.get(map_index=None)` and keeps a single sentinel for 
the unmapped case.



##########
airflow-ctl/docs/images/output_main.svg:
##########


Review Comment:
   It looks like two commands are added: `tasks state` and `taskinstances get`.
   
   <img width="1354" height="582" alt="Image" 
src="https://github.com/user-attachments/assets/fc34072c-c529-4fdf-b31b-652acbe3bc96";
 />



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to