hkc-8010 commented on code in PR #68012:
URL: https://github.com/apache/airflow/pull/68012#discussion_r4000173021
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py:
##########
@@ -309,6 +309,8 @@ def ti_run(
xcom_keys_to_clear=xcom_keys,
should_retry=_is_eligible_to_retry(previous_state, ti.try_number,
ti.max_tries),
)
+ if first_task_reschedule_start_date is not None:
+ context.first_task_reschedule_start_date =
first_task_reschedule_start_date
Review Comment:
I kept this bound after construction on purpose. The route is
`response_model_exclude_unset=True`, so passing the field to the constructor
marks it set and every task-run response then carries
`"first_task_reschedule_start_date": null`, not just the rescheduled ones. It
also breaks three exact-equality assertions in
`versions/head/test_task_instances.py` (lines 288, 857, 932) that this PR
otherwise doesn't touch. `arg_bindings` a few lines below uses the same
conditional-set pattern for the same reason.
Happy to switch if you'd rather have the field always present.
##########
airflow-core/tests/unit/api_fastapi/execution_api/versions/v2026_10_30/test_task_instances.py:
##########
@@ -17,10 +17,14 @@
from __future__ import annotations
+from uuid import uuid4
Review Comment:
This one is generating a `dag_id` string rather than an entity id. The
sibling `versions/head/test_task_instances.py` uses `str(uuid4())` for `dag_id`
in about 15 places, so I'd rather stay consistent with that file. Can change it
if you feel strongly.
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -690,6 +690,12 @@ def get_first_reschedule_date(self, context: Context) ->
AwareDatetime | None:
# If the task has not been rescheduled, there is no need to ask
the supervisor
return None
+ if from_server := self._ti_context_from_server:
+ # Servers older than the 2026-10-30 API version omit this, so fall
through to
+ # asking the supervisor below.
+ if (first_reschedule_date :=
from_server.first_task_reschedule_start_date) is not None:
+ return first_reschedule_date
Review Comment:
This is the same shape as what's already there, just with the walrus target
inlined. Inlining it puts the `if` at 116 characters and the comment at 118,
both over the 110 limit, so ruff-format splits them again. Keeping the
`from_server` binding reads better at that width so I left it alone.
--
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]
