1fanwang commented on code in PR #71763:
URL: https://github.com/apache/airflow/pull/71763#discussion_r4127679648
##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -346,6 +346,20 @@ def _load_help_texts_yaml() -> dict[str, dict[str, str]]:
help="Mapped task index",
)
+# Task logs command args. Required primitive parameters stay positional per the
+# airflowctl parameter style consensus (#66768).
+ARG_TASKS_LOGS_DAG_RUN_ID = Arg(
+ flags=("dag_run_id",),
+ type=str,
+ help="The run ID of the Dag run",
+)
+ARG_TRY_NUMBER = Arg(
+ flags=("--try-number",),
+ type=int,
+ default=-1,
+ help="The try number of the task instance logs to fetch; -1 fetches the
latest attempt",
+)
Review Comment:
```suggestion
ARG_TRY_NUMBER = Arg(
flags=("--try-number",),
type=int,
required=True,
help="The try number of the task instance logs to fetch",
)
```
`/logs/{try_number}` types that path param as `NonNegativeInt` and filters
`TaskInstance.try_number == try_number`, so `-1` is rejected by validation
before the handler runs. Against a real cluster:
```
$ curl -s -w '\nHTTP=%{http_code}\n' -H "Authorization: Bearer $TOKEN" \
-H "Accept: application/json" \
"$URL/api/v2/dags/nonexistent_dag_x/dagRuns/nonexistent_run_x/taskInstances/nonexistent_ti_x/logs/1?map_index=-1"
{"detail":"TaskInstance not found"}
HTTP=404
$ ... /logs/-1?map_index=-1
{"detail":[{"type":"greater_than_equal","loc":["path","try_number"],"msg":"Input
should be greater than or equal to 0","input":"-1","ctx":{"ge":0}}]}
HTTP=422
```
The `404` on the first call is the useful half: it shows the path and auth
are right and the handler ran, so the `422` is the endpoint rejecting
`try_number` itself, not a routing miss.
`required=True` matches the endpoint, and the `externalLogUrl` sibling route
is stricter still (`PositiveInt`), so there is no server-side "latest" to fall
back on. Keeping the default is still workable if you resolve it client-side
off `taskinstances get`, at the cost of one extra call.
Two spots to update alongside it: `test_logs_defaults_to_latest_attempt` and
the `test_airflowctl_commands.py:104` line both rely on the default, so they
will need an explicit `--try-number`.
--
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]
