You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/07/17 17:41:12 UTC
(superset) branch master updated: fix(ci): retry change_detector GitHub API calls on transient errors (#42138)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 905b20a3f2c fix(ci): retry change_detector GitHub API calls on transient errors (#42138)
905b20a3f2c is described below
commit 905b20a3f2cabcbac1c16741e4fb1078747a6e50
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Fri Jul 17 10:41:05 2026 -0700
fix(ci): retry change_detector GitHub API calls on transient errors (#42138)
Co-authored-by: Claude Opus 4.8 (1M context) <no...@anthropic.com>
---
scripts/__init__.py | 16 ++++
scripts/change_detector.py | 39 +++++++++-
tests/unit_tests/scripts/change_detector_test.py | 96 ++++++++++++++++++++++++
3 files changed, 147 insertions(+), 4 deletions(-)
diff --git a/scripts/__init__.py b/scripts/__init__.py
new file mode 100644
index 00000000000..13a83393a91
--- /dev/null
+++ b/scripts/__init__.py
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
diff --git a/scripts/change_detector.py b/scripts/change_detector.py
index 54ef039faf7..55548b270b0 100755
--- a/scripts/change_detector.py
+++ b/scripts/change_detector.py
@@ -20,9 +20,21 @@ import json
import os
import re
import subprocess
+import time
from typing import List, Optional
+from urllib.error import HTTPError, URLError
from urllib.request import Request, urlopen
+# The change detector gates the entire CI matrix, so a single transient GitHub
+# API hiccup should not fail the build. Retry server errors and network blips
+# with exponential backoff before giving up.
+MAX_RETRIES: int = 4
+RETRY_BACKOFF_SECONDS: int = 2
+REQUEST_TIMEOUT_SECONDS: int = 30
+# GitHub returns 429 (and 403 for secondary rate limits) when throttling, which
+# is transient and worth retrying alongside 5xx server errors.
+RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({403, 429})
+
# Define patterns for each group of files you're interested in
PATTERNS = {
"python": [
@@ -57,15 +69,34 @@ GITHUB_TOKEN = os.environ.get("GITHUB_TOKEN")
def fetch_files_github_api(url: str): # type: ignore
- """Fetches data using GitHub API."""
+ """Fetches data using GitHub API, retrying on transient failures."""
req = Request(url) # noqa: S310
req.add_header("Authorization", f"Bearer {GITHUB_TOKEN}")
req.add_header("Accept", "application/vnd.github.v3+json")
print(f"Fetching from {url}")
- with urlopen(req) as response: # noqa: S310
- body = response.read()
- return json.loads(body)
+ for attempt in range(1, MAX_RETRIES + 1):
+ try:
+ with urlopen(req, timeout=REQUEST_TIMEOUT_SECONDS) as response: # noqa: S310
+ body = response.read()
+ return json.loads(body)
+ except (HTTPError, URLError) as err:
+ # Retry transient failures: network errors (URLError has no status
+ # code), 5xx server errors, and GitHub rate-limit responses. Other
+ # 4xx client errors are deterministic, so re-raise immediately. Also
+ # re-raise once the retry budget is exhausted.
+ status = getattr(err, "code", None)
+ is_transient = (
+ status is None or status >= 500 or status in RETRYABLE_STATUS_CODES
+ )
+ if not is_transient or attempt == MAX_RETRIES:
+ raise
+ wait = RETRY_BACKOFF_SECONDS * 2 ** (attempt - 1)
+ print(
+ f"Attempt {attempt}/{MAX_RETRIES} failed ({err}); "
+ f"retrying in {wait}s..."
+ )
+ time.sleep(wait)
def fetch_changed_files_pr(repo: str, pr_number: str) -> List[str]:
diff --git a/tests/unit_tests/scripts/change_detector_test.py b/tests/unit_tests/scripts/change_detector_test.py
new file mode 100644
index 00000000000..69cf4dccce2
--- /dev/null
+++ b/tests/unit_tests/scripts/change_detector_test.py
@@ -0,0 +1,96 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from unittest import mock
+from urllib.error import HTTPError, URLError
+
+import pytest
+
+from scripts import change_detector
+
+
+def _make_response(body: bytes) -> mock.MagicMock:
+ """Builds a mock that mimics urlopen's context-manager response."""
+ response: mock.MagicMock = mock.MagicMock()
+ response.__enter__.return_value.read.return_value = body
+ return response
+
+
+def test_fetch_retries_transient_error_then_succeeds() -> None:
+ payload: list[dict[str, str]] = [{"filename": "superset/foo.py"}]
+ side_effects: list[object] = [
+ HTTPError("http://api", 500, "Server Error", {}, None), # type: ignore
+ HTTPError("http://api", 502, "Bad Gateway", {}, None), # type: ignore
+ _make_response(b'[{"filename": "superset/foo.py"}]'),
+ ]
+ with (
+ mock.patch.object(
+ change_detector, "urlopen", side_effect=side_effects
+ ) as urlopen_mock,
+ mock.patch.object(change_detector.time, "sleep") as sleep_mock,
+ ):
+ result = change_detector.fetch_files_github_api("http://api")
+
+ assert result == payload
+ assert urlopen_mock.call_count == 3
+ assert sleep_mock.call_count == 2
+
+
+def test_fetch_retries_rate_limit_then_succeeds() -> None:
+ side_effects: list[object] = [
+ HTTPError("http://api", 429, "Too Many Requests", {}, None), # type: ignore
+ _make_response(b"[]"),
+ ]
+ with (
+ mock.patch.object(
+ change_detector, "urlopen", side_effect=side_effects
+ ) as urlopen_mock,
+ mock.patch.object(change_detector.time, "sleep") as sleep_mock,
+ ):
+ result = change_detector.fetch_files_github_api("http://api")
+
+ assert result == []
+ assert urlopen_mock.call_count == 2
+ assert sleep_mock.call_count == 1
+
+
+def test_fetch_does_not_retry_client_error() -> None:
+ with (
+ mock.patch.object(
+ change_detector,
+ "urlopen",
+ side_effect=HTTPError("http://api", 404, "Not Found", {}, None), # type: ignore
+ ) as urlopen_mock,
+ mock.patch.object(change_detector.time, "sleep") as sleep_mock,
+ ):
+ with pytest.raises(HTTPError):
+ change_detector.fetch_files_github_api("http://api")
+
+ assert urlopen_mock.call_count == 1
+ assert sleep_mock.call_count == 0
+
+
+def test_fetch_gives_up_after_max_retries() -> None:
+ with (
+ mock.patch.object(
+ change_detector, "urlopen", side_effect=URLError("connection reset")
+ ) as urlopen_mock,
+ mock.patch.object(change_detector.time, "sleep"),
+ ):
+ with pytest.raises(URLError):
+ change_detector.fetch_files_github_api("http://api")
+
+ assert urlopen_mock.call_count == change_detector.MAX_RETRIES