This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new d22f4e4b470 Retry GitHub server errors in the fork CI monitor (#75692)
d22f4e4b470 is described below
commit d22f4e4b4707f74bed56de1f2154a0285a43a3f5
Author: Jarek Potiuk <[email protected]>
AuthorDate: Mon Oct 26 20:43:15 2026 +0100
Retry GitHub server errors in the fork CI monitor (#75692)
A single HTTP 504 from GitHub's GraphQL search, which happens when a search
takes too long on GitHub's side, failed the whole Fork CI monitor run before
it reconciled any pull request. Such errors usually go away on the next try,
like the rate limits the monitor already waits out.
Generated-by: Claude Opus 5
---
.../doc/ci/08_fork_ci_for_external_contributors.md | 9 +++--
.../src/airflow_breeze/utils/fork_ci_github.py | 42 ++++++++++++++++++----
dev/breeze/tests/test_fork_ci_github.py | 37 +++++++++++++++++++
3 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
index 6335400d128..ab33c09aec3 100644
--- a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
+++ b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
@@ -868,7 +868,8 @@ in their own section of the message so they are not
mistaken for reconciler fail
Unknown means any of:
-- GraphQL or REST error that is not a rate-limit wait (rate limits back off
and retry).
+- GraphQL or REST error that is not a rate limit or a GitHub server error, or
one of those
+ that outlasted its retries.
- A check suite for `Tests (AMD)`, in the fork or in `apache/airflow`, with a
status or
conclusion outside the modelled set.
- A workflow state outside `active` / `disabled_fork` / `disabled_manually` /
not found.
@@ -927,8 +928,10 @@ budget stays observable.
describe changes it did not make, a second Slack step that reports the first
of consecutive
runs failing before they could write their message, and saving what Slack
accepted to the
cache.
-- Every `gh` call times out after 120 seconds, and a rate limit is retried
with backoff up to
- four attempts, without waiting after the last one.
+- Every `gh` call times out after 120 seconds, and a rate limit or a GitHub
server error
+ (HTTP 5xx, such as a GraphQL search ending in 504) is retried with backoff
up to four
+ attempts, without waiting after the last one. Creating a comment is not
retried after a
+ server error, because the comment may have been created anyway.
The workflow does not exit early when `enabled` is false, because both pilots
rely on it.
With `enabled = false`, an empty `[pilot]` and no `use fork ci` labels, every
pull request
diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py
b/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py
index 73a44503acb..df00485a2b1 100644
--- a/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py
+++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py
@@ -19,6 +19,7 @@
from __future__ import annotations
import json
+import re
import subprocess
import time
from collections import Counter
@@ -31,6 +32,10 @@ from airflow_breeze.utils.fork_ci_messages import
STATUS_CONTEXT
RATE_LIMIT_MARKERS = ("rate limit", "secondary rate", "abuse detection")
MAX_ATTEMPTS = 4
+# GitHub answers with these while overloaded, and GraphQL searches that take
too long end in HTTP 504;
+# the same call usually succeeds shortly after.
+SERVER_ERROR = re.compile(r"\bHTTP 5\d\d\b")
+SERVER_ERROR_WAIT_SECONDS = 10
# A hung `gh` would otherwise hold the job until its timeout-minutes.
GH_TIMEOUT_SECONDS = 120
@@ -46,14 +51,23 @@ class GitHubClient:
self.calls: Counter[str] = Counter()
def _gh(
- self, args: Sequence[str], body: dict[str, Any] | None, kind: str, *,
allow_error_output: bool = False
+ self,
+ args: Sequence[str],
+ body: dict[str, Any] | None,
+ kind: str,
+ *,
+ allow_error_output: bool = False,
+ retry_server_errors: bool = True,
) -> Any:
"""
:param allow_error_output: return the JSON output of a failed call
when it has any. ``gh api
graphql`` exits non-zero when some fields of the response errored,
with the data GitHub
could resolve still on stdout.
+ :param retry_server_errors: repeat a call that failed with a server
error. Off for a call that
+ may have taken effect anyway and must not be repeated, such as
creating a comment.
"""
payload = json.dumps(body) if body is not None else None
+ rate_limited = False
for attempt in range(MAX_ATTEMPTS):
self.calls[kind] += 1
try:
@@ -69,11 +83,13 @@ class GitHubClient:
raise GitHubError(f"gh {' '.join(args)} timed out after
{GH_TIMEOUT_SECONDS}s")
if result.returncode == 0:
return json.loads(result.stdout) if result.stdout.strip() else
None
- if any(marker in result.stderr.lower() for marker in
RATE_LIMIT_MARKERS):
+ rate_limited = any(marker in result.stderr.lower() for marker in
RATE_LIMIT_MARKERS)
+ if rate_limited or (retry_server_errors and
SERVER_ERROR.search(result.stderr)):
if attempt == MAX_ATTEMPTS - 1:
break
- wait = 30 * 2**attempt
- console_print(f"[warning]GitHub rate limit, retrying in
{wait}s[/]")
+ reason = "rate limit" if rate_limited else "server error"
+ wait = (30 if rate_limited else SERVER_ERROR_WAIT_SECONDS) *
2**attempt
+ console_print(f"[warning]GitHub {reason}, retrying in
{wait}s[/]")
time.sleep(wait)
continue
if allow_error_output and result.stdout.strip():
@@ -82,7 +98,11 @@ class GitHubClient:
except json.JSONDecodeError:
pass
raise GitHubError(f"gh {' '.join(args)} failed:
{result.stderr.strip()}")
- raise GitHubError(f"gh {' '.join(args)} still rate limited after
{MAX_ATTEMPTS} attempts")
+ if rate_limited:
+ raise GitHubError(f"gh {' '.join(args)} still rate limited after
{MAX_ATTEMPTS} attempts")
+ raise GitHubError(
+ f"gh {' '.join(args)} still failing after {MAX_ATTEMPTS} attempts:
{result.stderr.strip()}"
+ )
def graphql(
self, query: str, variables: dict[str, Any] | None = None, *,
allow_partial: bool = False
@@ -123,12 +143,19 @@ class GitHubClient:
"""GET every page of a paginated REST path, keeping page-level fields
such as ``total_count``."""
return self._gh(["api", path, "--paginate", "--slurp"], None, "rest")
- def _mutate(self, description: str, args: Sequence[str], body: dict[str,
Any] | None) -> None:
+ def _mutate(
+ self,
+ description: str,
+ args: Sequence[str],
+ body: dict[str, Any] | None,
+ *,
+ retry_server_errors: bool = True,
+ ) -> None:
if self.dry_run:
console_print(f"[info][dry-run] {description}[/]")
return
console_print(f"[info]{description}[/]")
- self._gh(args, body, "mutation")
+ self._gh(args, body, "mutation",
retry_server_errors=retry_server_errors)
def add_labels(self, pr: int, labels: Sequence[str]) -> None:
self._mutate(
@@ -149,6 +176,7 @@ class GitHubClient:
f"#{pr}: create comment",
["api", "-X", "POST", f"repos/{self.repo}/issues/{pr}/comments"],
{"body": body},
+ retry_server_errors=False,
)
def update_comment(self, pr: int, comment_id: int, body: str) -> None:
diff --git a/dev/breeze/tests/test_fork_ci_github.py
b/dev/breeze/tests/test_fork_ci_github.py
index d993add8bb5..1bd85bcb749 100644
--- a/dev/breeze/tests/test_fork_ci_github.py
+++ b/dev/breeze/tests/test_fork_ci_github.py
@@ -53,6 +53,43 @@ def
test_gives_up_after_the_last_attempt_without_waiting(mock_run, mock_sleep):
assert [c.args[0] for c in mock_sleep.call_args_list] == [30, 60, 120]
[email protected]("stderr", ["gh: HTTP 504", "gh: Server Error (HTTP
502)"])
[email protected]("airflow_breeze.utils.fork_ci_github.time.sleep", autospec=True)
[email protected]("airflow_breeze.utils.fork_ci_github.subprocess.run",
autospec=True)
+def test_retries_on_server_errors(mock_run, mock_sleep, stderr):
+ mock_run.side_effect = [
+ _completed(1, stderr=stderr),
+ _completed(1, stderr=stderr),
+ _completed(0, stdout=json.dumps({"data": {"ok": True}})),
+ ]
+ client = GitHubClient()
+
+ assert client.graphql("query { ok }") == {"ok": True}
+ assert client.calls["graphql"] == 3
+ assert [c.args[0] for c in mock_sleep.call_args_list] == [10, 20]
+
+
[email protected]("airflow_breeze.utils.fork_ci_github.time.sleep", autospec=True)
[email protected]("airflow_breeze.utils.fork_ci_github.subprocess.run",
autospec=True)
+def test_gives_up_on_a_lasting_server_error(mock_run, mock_sleep):
+ mock_run.return_value = _completed(1, stderr="gh: HTTP 504")
+
+ with pytest.raises(GitHubError, match="still failing after 4 attempts: gh:
HTTP 504"):
+ GitHubClient().rest_get("repos/x/y")
+ assert [c.args[0] for c in mock_sleep.call_args_list] == [10, 20, 40]
+
+
[email protected]("airflow_breeze.utils.fork_ci_github.time.sleep", autospec=True)
[email protected]("airflow_breeze.utils.fork_ci_github.subprocess.run",
autospec=True)
+def test_does_not_repeat_creating_a_comment_after_a_server_error(mock_run,
mock_sleep):
+ mock_run.return_value = _completed(1, stderr="gh: Server Error (HTTP 502)")
+
+ with pytest.raises(GitHubError, match="502"):
+ GitHubClient().create_comment(10, "body")
+ mock_run.assert_called_once()
+ mock_sleep.assert_not_called()
+
+
@mock.patch(
"airflow_breeze.utils.fork_ci_github.subprocess.run",
autospec=True,
