This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch fork-ci-retry-server-errors in repository https://gitbox.apache.org/repos/asf/airflow.git
commit c9967be1e6f09367147c668525500ca414165f43 Author: Jarek Potiuk <[email protected]> AuthorDate: Mon Oct 26 19:52:05 2026 +0100 Retry GitHub server errors in the fork CI monitor 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,
