potiuk commented on code in PR #71206:
URL: https://github.com/apache/airflow/pull/71206#discussion_r4066600805
##########
airflow-ctl/src/airflowctl/ctl/commands/task_command.py:
##########
@@ -149,3 +156,31 @@ def states_for_dag_run(args, api_client=NEW_API_CLIENT) ->
None:
data=[_format_task_instance(ti, has_mapped_instances) for ti in
task_instances],
output=args.output,
)
+
+
+@provide_api_client(kind=ClientKind.CLI)
+def state(args, api_client=NEW_API_CLIENT) -> None:
+ """Get the state of a task instance."""
+ if (args.run_id is None) == (args.logical_date is None):
+ rich.print("[red]Provide either run_id or --logical-date, but not
both[/red]")
+ sys.exit(1)
+
+ run_id = args.run_id or _find_run_id_by_logical_date(api_client,
args.dag_id, args.logical_date)
Review Comment:
Heads-up rather than a change request: this selector block is now the third
copy in this file — identical to lines 89-94 in `failed_deps` and 138-143 in
`states_for_dag_run`.
#70904 is open and does exactly the job of collapsing all of them into a
shared `resolve_dag_run_id(api_client, args)`, so the two PRs will conflict and
whichever lands second needs a small rework. My instinct is that this one
should go first and #70904 then absorbs all three call sites in a single pass —
no action needed from you, I'll make sure both sides know.
One gentle observation while I'm here: you extracted
`_task_instance_not_found_message` to remove the duplicated 404 message, which
was the right call — the same instinct applied to this block would have avoided
the collision entirely.
##########
airflow-ctl/src/airflowctl/ctl/commands/task_command.py:
##########
@@ -149,3 +156,31 @@ def states_for_dag_run(args, api_client=NEW_API_CLIENT) ->
None:
data=[_format_task_instance(ti, has_mapped_instances) for ti in
task_instances],
output=args.output,
)
+
+
+@provide_api_client(kind=ClientKind.CLI)
+def state(args, api_client=NEW_API_CLIENT) -> None:
+ """Get the state of a task instance."""
+ if (args.run_id is None) == (args.logical_date is None):
+ rich.print("[red]Provide either run_id or --logical-date, but not
both[/red]")
+ sys.exit(1)
+
+ run_id = args.run_id or _find_run_id_by_logical_date(api_client,
args.dag_id, args.logical_date)
+
+ try:
+ task_instance = api_client.task_instances.get(
+ dag_id=args.dag_id,
+ dag_run_id=run_id,
+ task_id=args.task_id,
+ map_index=args.map_index,
+ suppress_error_log=True,
+ )
+ except ServerResponseError as e:
+ if e.response.status_code == 404:
+ rich.print(
+ f"[red]{_task_instance_not_found_message(args.dag_id, run_id,
args.task_id, args.map_index)}[/red]"
+ )
+ sys.exit(1)
+ raise
+
+ print(task_instance.state.value if task_instance.state else None)
Review Comment:
When a task instance has no state this prints the literal string `None`, and
`test_state_prints_none_when_task_instance_has_no_state` pins it, so I read
this as deliberate rather than an oversight.
Still worth a second thought: `None` is a Python repr surfacing in output
your description designs to be "script friendly". Every other value this
command emits is a lowercase state token (`success`, `running`, …), so a shell
caller comparing strings gets one odd capitalised Python-shaped value in the
set. An empty line, or a lowercase `none`, would sit better with the rest.
Either choice is defensible — if you keep `None`, a line in the command
description saying what an unset state prints would save the next person
working it out from the tests.
--
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]
