You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "henry3260 (via GitHub)" <gi...@apache.org> on 2026/04/07 20:10:14 UTC
[PR] CI: Avoid false recovery alerts when failed job lookup fails [airflow]
henry3260 opened a new pull request, #64863:
URL: https://github.com/apache/airflow/pull/64863
## Why
The CI notification flow currently fails open when it cannot fetch failed jobs for a failed workflow run. When `gh run view` fails, `workflow_status.py` falls back to an empty failed-jobs output, and the downstream notification logic interprets that as “no current failures.”
This can incorrectly mark an actually failing run as recovered, or skip notifications entirely.
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [ ] Yes (please specify the tool below)
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid false recovery alerts when failed job lookup fails [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #64863:
URL: https://github.com/apache/airflow/pull/64863
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid false recovery alerts when failed job lookup fails [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #64863:
URL: https://github.com/apache/airflow/pull/64863#issuecomment-4274553638
### Backport successfully created: v3-2-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-2-test</td>
<td><a href="https://github.com/apache/airflow/pull/65473"><img src="https://img.shields.io/badge/PR-65473-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid false recovery alerts when failed job lookup fails [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #64863:
URL: https://github.com/apache/airflow/pull/64863#issuecomment-4270515296
I think this one is good to go :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid false recovery alerts when failed job lookup fails [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #64863:
URL: https://github.com/apache/airflow/pull/64863#discussion_r3066496171
##########
dev/breeze/src/airflow_breeze/utils/workflow_status.py:
##########
@@ -93,7 +94,7 @@ def get_failed_jobs(run_id: int) -> list[str]:
)
if result.returncode != 0:
console.print(f"[red]Error fetching failed jobs: {result.stderr}[/red]")
- return []
+ return [FAILED_JOBS_LOOKUP_ERROR]
Review Comment:
Returning a human-readable error string inside `list[str]` mixes two different concepts (job names vs. lookup failure) and can confuse downstream consumers that expect real job identifiers (e.g., rendering, comparisons, or any logic that assumes entries map to actual jobs). A more robust approach is to represent lookup failure explicitly (e.g., raise an exception that the caller catches and treats as 'unknown', or change the return type to `list[str] | None` / a small result object like `{jobs: list[str], lookup_ok: bool}`), so the notification logic can avoid false recovery without treating an error message as a job.
##########
dev/breeze/src/airflow_breeze/utils/workflow_status.py:
##########
@@ -31,6 +31,7 @@
from rich.console import Console
console = Console(width=400, color_system="standard")
+FAILED_JOBS_LOOKUP_ERROR = "ERROR: failed to fetch failed jobs; inspect the workflow run directly"
Review Comment:
This message is likely user-facing (it may be displayed wherever failed jobs are shown) but it lacks actionable context such as the run id and/or the command that failed. Consider including the `run_id` (and/or a short hint like `gh run view <run_id> --json jobs`) in the propagated error so the recipient can immediately inspect the right workflow run without searching.
##########
dev/breeze/src/airflow_breeze/utils/workflow_status.py:
##########
@@ -31,6 +31,7 @@
from rich.console import Console
console = Console(width=400, color_system="standard")
+FAILED_JOBS_LOOKUP_ERROR = "ERROR: failed to fetch failed jobs; inspect the workflow run directly"
Review Comment:
Returning a human-readable error string inside `list[str]` mixes two different concepts (job names vs. lookup failure) and can confuse downstream consumers that expect real job identifiers (e.g., rendering, comparisons, or any logic that assumes entries map to actual jobs). A more robust approach is to represent lookup failure explicitly (e.g., raise an exception that the caller catches and treats as 'unknown', or change the return type to `list[str] | None` / a small result object like `{jobs: list[str], lookup_ok: bool}`), so the notification logic can avoid false recovery without treating an error message as a job.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org