Compare commits

...
Author SHA1 Message Date
emilandDevin <158243242+devin-ai-integration[bot]@users.noreply.github.com> dd603d3e91 fix: add tenacity retry to install_tools._download for transient network failures
The build-images CI job failed because GitHub releases dropped
connections ("Remote end closed connection without response") for
actionlint, vale, and hadolint simultaneously. The previous code only
fell through to the next fallback URL — there was no retry on the same
URL for transient connection resets.

Now uses tenacity.Retrying with exponential backoff (2-10s, 3 attempts)
on URLError/OSError/ConnectionError, matching the pattern already used
in devx.utils.network. The _sleep kwarg allows tests to skip real
sleeping.

3 new tests cover: retry-then-success, exhaustion-after-max-retries,
and no-retry-on-non-transient-errors.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-08-12 20:13:03 +02:00
2 changed files with 105 additions and 2 deletions
+43 -2
View File
@@ -22,18 +22,37 @@ Usage::
from __future__ import annotations
import logging
import os
import platform
import shutil
import tarfile
import tempfile
import time
import urllib.error
import urllib.request
from pathlib import Path
import click
from tenacity import (
Retrying,
before_sleep_log,
retry_if_exception_type,
stop_after_attempt,
wait_exponential,
)
TARGET_DIR = Path.home() / ".local" / "bin"
logger = logging.getLogger(__name__)
# Retry configuration for transient network failures during download.
# GitHub releases occasionally drops connections ("Remote end closed
# connection without response"). Retrying with backoff before falling
# through to the next fallback URL makes the build resilient to
# momentary network blips.
MAX_DOWNLOAD_RETRIES = 3
ACTIONLINT_VERSION = "1.7.12"
GIT_CLIFF_VERSION = "2.13.1"
@@ -64,8 +83,30 @@ def _ensure_target_dir() -> Path:
return TARGET_DIR
def _download(url: str, dest: Path) -> None:
"""Download a file from ``url`` to ``dest`` with a 60s timeout."""
def _download(url: str, dest: Path, *, _sleep=None) -> None:
"""Download a file from ``url`` to ``dest`` with retry and 60s timeout.
Retries up to ``MAX_DOWNLOAD_RETRIES`` times on transient network
errors (``URLError``, ``OSError`` from connection resets) using
exponential backoff. This handles momentary GitHub releases
connection drops that were causing CI image builds to fail.
The ``_sleep`` kwarg is for tests to avoid real sleeping; production
code should leave it as ``None`` (uses ``time.sleep``).
"""
retrying = Retrying(
stop=stop_after_attempt(MAX_DOWNLOAD_RETRIES),
wait=wait_exponential(multiplier=2, min=2, max=10),
retry=retry_if_exception_type((urllib.error.URLError, OSError, ConnectionError)),
before_sleep=before_sleep_log(logger, logging.WARNING),
sleep=_sleep if _sleep is not None else time.sleep,
reraise=True,
)
retrying(_do_download, url, dest)
def _do_download(url: str, dest: Path) -> None:
"""Single download attempt — called by :func:`_download` retry wrapper."""
with urllib.request.urlopen(url, timeout=60) as resp, open(dest, "wb") as f: # nosec B310
shutil.copyfileobj(resp, f)
+62
View File
@@ -1,6 +1,7 @@
from __future__ import annotations
import platform
import urllib.error
from pathlib import Path
from unittest.mock import patch
@@ -68,6 +69,67 @@ class TestDownload:
mock_urlopen.assert_called_once()
assert dest.read_bytes() == b"data"
def test_download_retries_on_transient_error(self, tmp_path: Path) -> None:
"""Download retries on URLError then succeeds."""
dest = tmp_path / "file.bin"
call_count = [0]
class _FakeResponse:
def __init__(self) -> None:
self._sent = False
def __enter__(self) -> _FakeResponse:
return self
def __exit__(self, *args: object) -> None:
pass
def read(self, n: int = -1) -> bytes:
if self._sent:
return b""
self._sent = True
return b"data"
def _flaky_urlopen(url: str, timeout: int = 60):
call_count[0] += 1
if call_count[0] < 2:
raise urllib.error.URLError("Remote end closed connection")
return _FakeResponse()
with patch("urllib.request.urlopen", side_effect=_flaky_urlopen):
install_tools._download("https://example.com/file", dest, _sleep=lambda _: None)
assert call_count[0] == 2
assert dest.read_bytes() == b"data"
def test_download_fails_after_max_retries(self, tmp_path: Path) -> None:
"""Download raises after MAX_DOWNLOAD_RETRIES attempts."""
dest = tmp_path / "file.bin"
call_count = [0]
def _always_fail(url: str, timeout: int = 60):
call_count[0] += 1
raise urllib.error.URLError("Remote end closed connection")
with patch("urllib.request.urlopen", side_effect=_always_fail):
with pytest.raises(urllib.error.URLError):
install_tools._download("https://example.com/file", dest, _sleep=lambda _: None)
assert call_count[0] == install_tools.MAX_DOWNLOAD_RETRIES
assert not dest.exists()
def test_download_no_retry_on_non_transient_error(self, tmp_path: Path) -> None:
"""Download does not retry on non-network errors (e.g. ValueError)."""
dest = tmp_path / "file.bin"
call_count = [0]
def _fail_with_value_error(url: str, timeout: int = 60):
call_count[0] += 1
raise ValueError("not a network error")
with patch("urllib.request.urlopen", side_effect=_fail_with_value_error):
with pytest.raises(ValueError):
install_tools._download("https://example.com/file", dest, _sleep=lambda _: None)
assert call_count[0] == 1
class TestDownloadBinary:
def test_download(self, tmp_path: Path) -> None: