Surface the retry policy decision on task instances page - #73030
amoghrajesh wants to merge 9 commits into
Conversation
boring-cyborg
Bot
added
area:airflow-ctl
area:API
|
|
||
| return ( | ||
| <Box p={2}> | ||
| {taskInstance?.retry_reason === null || taskInstance?.retry_reason === undefined ? undefined : ( |
There was a problem hiding this comment.
Nothing clears retry_reason when a task is cleared, so this banner outlives the state it describes. I ran clear_task_instances against a failed ti with the column set and got back state=None, retry_reason='PROBE-REASON auth error, do not retry': clear_task_instances resets state, external_executor_id and the next-method args but leaves the retry-policy columns alone, and the only reset is in ti_run when the task next enters RUNNING. For a paused dag, or a task queued behind a full pool, that window is indefinite and the page keeps showing an orange "Reason for state" quoting the previous attempt. Gating the banner on failed/up_for_retry, or clearing the column in clear_task_instances, would close it.
There was a problem hiding this comment.
Gated the banner on failed/up_for_retry, so a cleared task no longer shows a reason describing the previous attempt.
Leaving the column itself alone here and doing it as a separate PR, because the staleness predates both of these PRs and is wider than it first looks: clear_task_instances leaves retry_delay_override behind too, and that one is functional rather than cosmetic, next_retry_datetime reads it in preference to the task's configured retry_delay, so a policy supplied delay from an earlier run silently controls the timing of the next retry after a clear, even if the policy has since changed or been removed. That seems worth its own change and its own test against a function this heavily used, rather than riding along in a UI PR.
| <Box p={2}> | ||
| {taskInstance?.retry_reason === null || taskInstance?.retry_reason === undefined ? undefined : ( | ||
| <Alert | ||
| data-testid="retry-reason-alert" |
There was a problem hiding this comment.
No frontend test covers the banner or the row. TaskInstance.test.tsx and Header.test.tsx are right next door, and the branches worth pinning are cheap ones: banner absent when the reason is null, and error vs warning picked by state. This data-testid is already the hook for it.
There was a problem hiding this comment.
Added Details.test.tsx, 10 tests, using the Wrapper + i18n.t() convention from Header.test.tsx. Covers the two branches you named plus the state gating and the banner/row split.
| </Flex> | ||
| </Table.Cell> | ||
| </Table.Row> | ||
| {tryInstance?.retry_reason === null || tryInstance?.retry_reason === undefined ? undefined : ( |
There was a problem hiding this comment.
This row reads the selected try (tryInstance) while the banner at the top reads the latest try (taskInstance), and both are labelled taskInstance.retryReason. On the default view that prints the same string twice, since /tries/{n} returns the live ti for the current try number. Pick try 1 of 3 in the Tries strip and the table shows try 1's reason while the banner directly above the selector still shows try 3's, with nothing on screen distinguishing them. Should the banner follow tryInstance too?
There was a problem hiding this comment.
Kept the banner on the latest try but stopped it sharing the row's label, which is what made the two read as a duplicate on the default view and as an unexplained disagreement elsewhere. It now titles itself with the try counts - "Stopped on try 3 of 3", or "Retrying after try 2 of 3" - so it states which attempt it describes and the row underneath stays scoped to the selected try.
That also fills a gap the page had regardless of this feature: the retry limit was not shown anywhere, so "retries exhausted" in the reason text had nothing to anchor to.
| queued_by_job: JobResponse | None = Field(alias="triggerer_job") | ||
| dag_version: DagVersionResponse | None | ||
| team_name: str | None = None | ||
| retry_reason: str | None = None |
There was a problem hiding this comment.
This is the first time retry_reason becomes public v2 API surface, so the name is fixed from here on. I checked main: the field exists today only as the ORM column and inside the versioned execution API, and is absent from both v2-rest-api-generated.yaml and these datamodels, so this is the point of no return. #73027 writes it on the terminal-failure path as well, and this PR's own label is "Reason for state" rather than anything retry-shaped, so the name is already out of step with what the field carries. Worth naming the response field for that (state_reason?) while it is still free, even if the ORM column keeps retry_reason.
There was a problem hiding this comment.
If we are worried about locking in the name, we could expose this only on the /ui API but we haven't done any other custom endpoints for task instances yet.
"State reason" isn't bad though! I think theres been talk of something along those lines for a while. Then we could reuse this in the future and it could be a retry reason if the task has multiple tries.
There was a problem hiding this comment.
Renamed to state_reason on both TaskInstanceResponse and TaskInstanceHistoryResponse.
The two API tests still seed the fixture with retry_reason and assert state_reason comes back, so they cover the alias mapping itself rather than just the field's presence.
There was a problem hiding this comment.
If we are worried about locking in the name, we could expose this only on the /ui API but we haven't done any other custom endpoints for task instances yet.
state_reason seems good actually even in public API.
| return RetryTask(**retry_kwargs), TaskInstanceState.UP_FOR_RETRY | ||
| if retry_reason is not None and ti._ti_context_from_server is not None: | ||
| max_tries = ti._ti_context_from_server.max_tries | ||
| retry_reason = f"{retry_reason}; retries exhausted ({ti.try_number} of {max_tries})" |
There was a problem hiding this comment.
This renders as "retries exhausted (3 of 2)" in production. The branch is only reached when the server sent should_retry=False, and the server computes that as max_tries != 0 and try_number <= max_tries, so with retries configured you only arrive here once try_number == max_tries + 1. The denominator wants to be max_tries + 1, which is the convention this same file already uses at line 2137 (Try {{try_number}} out of {{max_tries + 1}}) and that models/taskinstance.py:1465 uses for Starting attempt %s of %s. There is also a max_tries == 0 case: a policy returning RETRY on a task with no retries configured renders "retries exhausted (1 of 0)", where there was no retry budget to exhaust.
There was a problem hiding this comment.
Fixed on #73027: the denominator is now max_tries + 1, matching Starting attempt %s of %s in core and the alert template in this same file. The max_tries == 0 case you flagged is handled too — the suffix is skipped entirely rather than rendering "(1 of 0)", since there was no budget to exhaust.
|
|
||
| assert state == TaskInstanceState.FAILED | ||
| assert isinstance(msg, TaskState) | ||
| assert msg.retry_reason == "rate limit; retries exhausted (2 of 2)" |
There was a problem hiding this comment.
This assertion only reads correctly because the fixture forces a combination the server never sends. create_runtime_ti(try_number=2, max_tries=2, should_retry=False) is unreachable in production: _is_eligible_to_retry("running", 2, 2) returns True, so the real server would have sent should_retry=True here. Actual exhaustion is try_number == max_tries + 1, which is what exposes the off-by-one in the message above. try_number=3, max_tries=2 would match what the server produces, and two distinct values would also pin the argument order, which 2 of 2 cannot.
There was a problem hiding this comment.
Fixed on #73027, and taken a bit further than the suggestion: rather than hardcoding try_number=3, max_tries=2, the operator now carries retries=2 and the fixture derives max_tries and should_retry itself. That keeps the triple inside the reachable space by construction rather than by convention, so it can't drift back out. Expected string is (3 of 3).
| if updated_state == TaskInstanceState.FAILED: | ||
| # This is the only case needs extra handling for TITerminalStatePayload | ||
| if isinstance(ti_patch_payload, TITerminalStatePayload) and ti_patch_payload.retry_reason: | ||
| failed_retry_reason: str | None = ti_patch_payload.retry_reason[:500] |
There was a problem hiding this comment.
Truncating from the tail drops the part this change adds. _finalize_task_failure composes the terminal reason as f"{retry_reason}; retries exhausted (...)" and, unlike the retry branch just below which caps with retry_reason[:500] before sending, never caps the composed string. So for a policy reason over roughly 470 characters the suffix is the first thing lost and what lands in the column is a mid-sentence cut of the reason. LLMRetryPolicy builds the reason from an unconstrained reasoning field, so long values are reachable rather than theoretical. Capping the reason before appending, or eliding the middle instead of the tail, would keep the more useful half.
There was a problem hiding this comment.
Fixed on #73027. _finalize_task_failure now caps the base reason at 500 - len(suffix) before appending, so the suffix survives any reason length; the [:500] here stays as a backstop for the plain FAIL path, which has no suffix. Pinned by a test using a 600-character reason that asserts both len == 500 and endswith("; retries exhausted (3 of 3)") — I checked it fails on the old form.
| assert ti.next_kwargs is None | ||
| assert ti.duration == 3600.00 | ||
|
|
||
| def test_ti_update_state_to_failed_persists_retry_reason(self, client, session, create_task_instance): |
There was a problem hiding this comment.
These three tests only exercise the head version, so nothing pins the version boundary this change introduces. AddTerminalStateRetryReasonField was added to Version("2026-10-30"), and TITerminalStatePayload is a StrictBaseModel with extra="forbid", so a terminal-state PATCH carrying retry_reason at Airflow-API-Version: 2026-06-30 should be rejected. The other two changes in that same version each have a boundary test (v2026_10_30/test_task_instances.py::TestArgBindingsFieldBackwardCompat and v2026_10_30/test_callbacks.py::TestRunCallbackEndpointVersioning); a short case alongside them would stop this version change from silently going missing.
There was a problem hiding this comment.
Superseded by your own follow-up on #73027 — you measured that cadwyn doesn't reach through the TIStateUpdate discriminated union, so the route validates against the head models whatever version is pinned, and the boundary test would fail today if written. Leaving it out on that basis. The supervisor-side gate does bind and is covered by TestRealBundleRetryReasonUpgrade in test_migrator.py.
bbovenzi
left a comment
bbovenzi
left a comment
There was a problem hiding this comment.
If this is all good on the datamodel side then I like it.
Kaxil's suggestion for state_reason makes sense to me though. Gives us more flexibility.
| "default": null, | ||
| "title": "Rendered Map Index" | ||
| }, | ||
| "retry_reason": { |
There was a problem hiding this comment.
The generated SDK mirrors of this schema didn't get regenerated, and static checks are red on one of them: check-ts-sdk-supervisor-schema exits 1 with "files were modified by this hook" (run 35580411680). That hook is deliberately skipped for schema-only changes so regeneration can be the ts-sdk follow-up's job, but skip_prek_hooks returns early once full_tests_needed is set (selective_checks.py:1708-1711), and this PR sets it by touching v2-rest-api-generated.yaml, which matches the API-codegen file group. The CI log confirms it: skip-prek-hooks: identity,update-uv-lock. cd ts-sdk && pnpm run generate:supervisor should be a two-line diff, and retry_reason is optional so nothing in ts-sdk/src/coordinator/ needs to change with it. The other two mirrors are stale the same way with no hook firing on this PR: go-sdk/pkg/execution/genmodels/models.gen.go and java-sdk/sdk/schema/schema.json both still have TaskState as state/end_date/type/rendered_map_index. Worth deciding whether they ride along here or in the follow-up.
There was a problem hiding this comment.
Split into three, because they turn out to be different situations.
ts-sdk - regenerated
java-sdk - I don't think this one is stale. It doesn't mirror our local schema at all: build.gradle.kts downloads a published schema at the version pinned in gradle.properties, currently 2026-06-16. Bumping it would need the 2026-10-30 schema published, which can't happen while that version is unreleased.
go-sdk - you're right that it's stale and fix is here: #73531
| </Flex> | ||
| </Table.Cell> | ||
| </Table.Row> | ||
| {tryInstance?.state_reason === null || tryInstance?.state_reason === undefined ? undefined : ( |
There was a problem hiding this comment.
The banner is gated on failed/up_for_retry now, but this row isn't; it renders on a non-null reason alone. After a clear the column survives, so the same staleness comes back here: clear_task_instances resets state, external_executor_id, the next-method args and max_tries and never touches retry_reason (taskinstance.py:444-447), and the page then shows State: (no status) with Reason for state: auth error, do not retry directly underneath it. Applying the same state gate here closes it without waiting on the column-clearing PR you described. The four cases at Details.test.tsx:129-137 set the reason on both objects but assert only the banner's absence, so they walk straight past this row; adding expect(screen.queryByText(i18n.t("common:taskInstance.stateReason"))).not.toBeInTheDocument() to them fails against today's code.
There was a problem hiding this comment.
Both now use the same list of states, kept in one place so they cannot drift apart again. Thats what let them get out of sync in the first place.
One difference from your suggestion: the row checks the selected try's state, not the task's. If it used the task's state, picking an older failed try while the task is running again would hide that try's reason, which is the whole point of the row. Your suggested assertion passes either way, so I added a test that tells the two apart.
| max_tries = ti._ti_context_from_server.max_tries | ||
| if max_tries > 0: | ||
| # max_tries is the retry count, not the attempt count -- total attempts is max_tries + 1. | ||
| suffix = f"; retries exhausted ({ti.try_number} of {max_tries + 1})" |
There was a problem hiding this comment.
This prints (3 of 3), and the banner title added in the same push already reads "Stopped on try 3 of 3", built from the same try_number and max_tries + 1 (Details.tsx:127). The alert ends up stating the counts twice.
There is a second reason to drop the suffix rather than reword it. retries defaults to 0, and _is_eligible_to_retry is max_tries != 0 and try_number <= max_tries, so a task without an explicit retries= reaches this branch with the suffix skipped. Where a policy returned RETRY, what gets stored is the bare reason: "Transient rate limit, backing off for 60s" on a task that is not going to retry. All four new tests pass retries=2, so the default case is untested.
Leaving the counts to the UI would cover both.
There was a problem hiding this comment.
Agreed, dropped it. The worker now stores only the policy's own words.
That fixes both things you raised: no more saying the counts twice, and no more bare "rate limit, backing off 60s" sitting on a task that was never going to retry. The whole max_tries branch is gone, so there's no longer a special case to get wrong.
The retries=0 case is now covered. The old test is parametrized over the two ways this branch is reached: budget exhausted (retries=2, try 3) and no budget at all (retries=0, try 1). Both expect the plain reason.
Counts are the UI's job now, which it already does in the banner.
| assert out["retry_reason"] is None | ||
|
|
||
| def test_upgrade_keeps_retry_reason_at_head(self, real_migrator): | ||
| from airflow.sdk.execution_time.comms import TaskState |
There was a problem hiding this comment.
Not reopening the core-side boundary test you closed. This is narrower: the gate here does work, but neither test in this class exercises the direction it governs. I checked both halves rather than reasoning from the code. Deleting AddRetryReasonToTaskState from the bundle leaves both tests green. A downgrade probe on the same build shows the instruction is live: downgrade(TaskState(..., retry_reason="auth error"), "2026-06-16") comes back without the key, while "2026-10-30" keeps it.
The mechanism matches that. schema(TaskState).field("retry_reason").didnt_exist is filed under cadwyn's alter_schema_instructions, not the alter_request_by_schema_instructions that SchemaVersionMigrator.upgrade iterates, and upgrade then validates against the head class, which always carries the field; the comment at migrator.py:159-162 says as much. test_upgrade_keeps_retry_reason_at_head also hits the source_version == supervisor_version early return, so it reduces to a pydantic round-trip.
A downgrade(TaskState(..., retry_reason="x"), "2026-06-16") asserting the key is absent would pin the version change, and fails once it is deleted. TestRealBundleArgBindingsDowngrade just above is the template. Minor: the two function-level from airflow.sdk.execution_time.comms import TaskState imports can move to the top of the file.
There was a problem hiding this comment.
You're right, neither test touched the direction the gate governs. Added a downgrade test: retry_reason is gone at 2026-06-16 and kept at 2026-10-30.
Checked it does what the old ones didn't. Deleting AddRetryReasonToTaskState from the bundle now fails that test, while the others stay green exactly as you measured.
Dropped test_upgrade_keeps_retry_reason_at_head as you pointed out it passes the supervisor's own version, so it short circuits and only proves pydantic round-trips. Kept the upgrade test that backfills None, since that's the real foreign-runtime path.
| below). On a RETRY the value is cleared once the next attempt starts running. | ||
| A FAIL is terminal, so there is no next attempt to clear it and the reason | ||
| stays on the row. When the model asked to retry but no attempts were left, the | ||
| stored reason ends with a ``; retries exhausted (N of M)`` note. |
There was a problem hiding this comment.
The ; retries exhausted (N of M) note is promised here without qualification, but task_runner.py skips it when max_tries <= 0, and that is the default retries=0 case.
The Requires Airflow >= 3.3.0 note at the top of the page has also gone stale for the FAIL path. That path needs TITerminalStatePayload.retry_reason, which lands in 3.4.0 (airflow-core/src/airflow/__init__.py reads 3.4.0, latest tag is 3.3.2), while the provider floors at apache-airflow>=3.0.0. On a 3.3.x deployment the sentence being removed here is still the accurate one.
One other gap: the page names only retry_reason, which is the name a user cannot see anywhere. grep -r state_reason providers/common/ai/docs airflow-core/docs comes back empty. Since the point of this change is making the reason discoverable, a line pointing at the REST field and the Details page would finish it.
There was a problem hiding this comment.
All three fixed.
|
|
||
| // Chakra encodes `status` in a generated class rather than a DOM attribute, so the error/warning | ||
| // distinction can only be pinned as "the two states do not render identically". | ||
| it("styles a failed banner differently from an up_for_retry one", () => { |
There was a problem hiding this comment.
We have a test for the banner but not the row. Which would make sure we avoid the issue Kaxil found in Details.tsx
There was a problem hiding this comment.
Agreed, and that gap is exactly what let the row bug through. Fixed.
The four "stale state" tests now check the row as well as the banner, and there's a new test for the case where an older try keeps its reason while the task runs again.
I checked the tests actually catch the bug rather than just passing: removing the row gate fails four of them, and gating the row on the wrong state fails the new one.
|
|
||
| assert state == TaskInstanceState.FAILED | ||
| assert isinstance(msg, TaskState) | ||
| assert msg.retry_reason == "rate limit; retries exhausted (3 of 3)" |
There was a problem hiding this comment.
Not reopening the suffix design, you already settled that on #73027. This is about branch state: git merge-base --is-ancestor 524f00dce2 6d467f3daa returns non-zero, so this head last picked up #73027 at cfd075de77 and does not contain the commit the three "Handled in 524f00d" replies point at. That leaves two of this PR's own tests asserting a string the parent has since deleted: this line expects "rate limit; retries exhausted (3 of 3)" and line 1268 expects .endswith("; retries exhausted (3 of 3)"), while 524f00dce2 replaced both with a parametrized pytest.param("retry_exhausted", 2, 3, id="budget-exhausted") expecting the bare reason. Both go red on merge-down, and the [: 500 - len(suffix)] arithmetic at task_runner.py:1943 goes dead with them.
retry_policies.rst here also still carries both sentences the parent corrected, the unconditional ; retries exhausted (N of M) promise at line 148 and the version note at line 22; and main has rewritten that file (+393/-69) since this branch's merge-base, moving the sentence this PR corrects to main:420, so the hunk will not apply as-is. Worth landing #73027 and rebasing before this one gets another review pass.
What I need before approving:
- Land Persist
retry_reasonnot just for retries but even when a task fails #73027 and rebase. That turns the two red assertions above green, brings in the suffix removal and the migrator downgrade test, and clears the current conflict with main (mergeableisfalseright now). - Redact
state_reasonbefore serving, on this model andtask_instance_history.py:65, the wayconnections.py:48does. With a test that amask_secret()-registered value does not come back in the response. - Make the new frontend tests capable of failing: register the i18n bundle, and make one try-count case non-degenerate. Detail in the
Details.test.tsxcomment. - Have
STATES_WITH_REASONbind both surfaces, or drop the redundant clause and correct the comment at 49 to 51. - CI actually green. Two checks have run on this head, so nothing is verified yet.
None of the rest blocks me. The wire field is still retry_reason on TITerminalStatePayload and TaskState while the REST field is now state_reason; aligning them is free while 2026-10-30 is unreleased and API_VERSION already points at it, and after release it costs a version change plus the ts and java mirrors, so it is worth settling now either way. test_ti_update_state_to_failed_without_retry_reason passes with both new route lines deleted because the fixture never sets the column, and seeding a stale value first would pin the preserve-on-absent behaviour that differs from the retry branch. _finalize_task_failure logs Retry policy decision with action="fail" after _evaluate_retry_policy already logged the same event name with action="retry", and the parent keeps that line so the merge-down will not resolve it. And retry_reason is accepted on skipped, removed and upstream_failed and then dropped, which a docstring saying the field is failed-only would close.
Clearing the columns in clear_task_instances is yours to do separately, and the execution-API boundary test stays closed.
| queued_by_job: JobResponse | None = Field(alias="triggerer_job") | ||
| dag_version: DagVersionResponse | None | ||
| team_name: str | None = None | ||
| state_reason: str | None = Field(default=None, validation_alias="retry_reason") |
There was a problem hiding this comment.
This is the line that makes the reason public, and it goes out with no masking. Two siblings in this same directory redact before serving: connections.py:48 has a @field_validator("password", mode="after") calling redact, and variables.py:41 does the same for val. Nothing masks it upstream either; redact() in task_runner.py is applied to rendered template fields only (1343/1360/1387), and the worker stores decision.reason verbatim.
The reachable paths are the documented ones. LLMRetryPolicy(redact_exception=False) is a supported opt-out (policies/retry.py:277), and with it the model reads the raw exception and can echo a credential into reasoning, which becomes the stored reason at retry.py:415/:424. A hand-written RetryPolicy returning reason=f"...{exc}" has the same shape, and that is the extension point the docs point people at. The same string in a task log would be masked, so this is the one surface where it is not. It also is not just the details page: _private_ui.yaml picks state_reason up on TaskInstanceResponse, so it rides the list endpoints too.
A redact validator on both this and task_instance_history.py:65 would match what connections.py already does.
| reason === null || | ||
| reason === undefined || | ||
| taskInstance === undefined || | ||
| !STATES_WITH_REASON.includes(taskInstance.state ?? "") |
There was a problem hiding this comment.
The shared constant does not actually bind the banner, so it does not prevent the drift it was introduced for. This includes check is redundant: the two inner branches at 139 and 147 already test state === "failed" and === "up_for_retry" as literals, with the return undefined at 154 catching everything else.
I checked by mutation rather than by reading. Four runs from a scratch copy of Details.tsx:
widen STATES_WITH_REASON 3 failed
force the row gate true 4 failed
banner error -> warning 1 failed
force this banner gate false 0 failed (11 passed)
So the constant only gates the row, and the comment at 49 to 51 saying both surfaces key off it is not true today. Add a state to the list and the row renders it while the banner falls through to undefined.
One as const record keyed by state holding {status, titleKey}, driving both the banner and the row, would make a new state fail to compile until it has a title. It would also fold the duplicated predicate at 158 to 165 into the same source.
| renderDetails(buildTaskInstance({ max_tries: 2, state, state_reason: "auth error", try_number: 3 })); | ||
|
|
||
| expect(screen.getByTestId("state-reason-alert")).toHaveTextContent( | ||
| i18n.t(`common:taskInstance.stateReasonSummary.${titleKey}`, { totalTries: 3, tryNumber: 3 }), |
There was a problem hiding this comment.
Neither of these title assertions can fail on a wrong count.
i18n is never initialized under vitest. src/i18n/config.ts only reaches .init() inside Promise.all([resolveI18nVersion(), resolveExtraLanguages()]).then(...), and resolveI18nVersion goes through VersionService.getVersion(), which does not resolve under the test server; testsSetup.ts registers no resource bundle. So i18n.t("common:taskInstance.stateReasonSummary.failed", {...}) hands back the key verbatim, and toHaveTextContent then compares that key against the same key rendered by the component. Interpolation values never enter it. DagDeactivatedBanner.test.tsx:75-76 already has the fix in this repo: i18n.addResourceBundle("en", "common", commonLocale, true, true) in a beforeEach.
The fixture is degenerate as well. Line 108 sets max_tries: 2, try_number: 3, so totalTries (max_tries + 1) and tryNumber are both 3, and the assertion cannot tell the two arguments apart. I ran both mutations from a scratch copy: reverting Details.tsx to interpolate a bare max_tries, which is the (3 of 2) off-by-one from round 1, leaves all 11 tests green, and so does swapping the two arguments.
An up_for_retry case with max_tries: 3, try_number: 2 expecting "Retrying after try 2 of 4" makes the counts distinguishable, and matches the state where try_number != max_tries + 1 is the norm rather than the exception. With the resource bundle registered as well, the off-by-one mutation fails 3 tests instead of none.
This branch has not been deployed
3 participants
Was generative AI tooling used to co-author this PR?
What
A retry policy can already classify why a task failed (auth error, rate limit, and so on), and #73027 makes sure that reason is actually saved to the database on the failure path, not just the retry path. But that reason still isn't visible anywhere - not through the API, not in the UI. This PR closes that gap: it exposes
retry_reasonthrough the public REST API and renders it on the Task Instance page, so a Dag author looking at a failed task can finally see a plain-English reason instead of just "FAILED" with no explanation. Stacked stacked on #73027.Current behaviour
retry_reasonis a real column on bothtask_instanceandtask_instance_history(written by #73027), but neither the public REST API'sTaskInstanceResponse/TaskInstanceHistoryResponsemodels nor the Task Instance UI expose it. The data sits in the database with no way to see it.Proposed change
Backend (core API):
retry_reason: str | None = Noneto bothTaskInstanceResponseandTaskInstanceHistoryResponse(airflow-core/src/airflow/api_fastapi/core_api/datamodels/). Both models needed the field - the Task Instance page's per try data comes from thetry-detailsendpoint, which returnsTaskInstanceHistoryResponsebacked by either the live row or an archived history row.GET .../taskInstances/{task_id}andGET .../tries/{try_number}already return the ORM object directly, so FastAPI/Pydantic auto-maps the existingretry_reasoncolumn.airflow-ctl's generated datamodels via their respective hooks (never hand-edited).Frontend (
Details.tsx, the Task Instance details page):Alertsystem-component (the same oneWarningAlert/ErrorAlertuse), so it's visually consistent with how errors are already surfaced elsewhere in the UI.taskInstance.retryReason: "Reason for state"); other locales fall back to English until translated separately, since the i18n validity check doesn't enforce cross-locale key parity.Testing
Ran the same
example_llm_retry_policydag.Try 1:
Try 2:
Try 3:
When in retry state:
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.