Skip to content

Commit 1635481

Browse files
lucia-sbclaude
andauthored
Retry wheel downloads in ddev size on dropped connections (#25247)
* Retry wheel downloads in ddev size on dropped connections The disk-usage measurement CI job downloads wheels from the internal wheels storage host to compute integration sizes. That host resets the connection under load often enough to fail the job several times a week, leaving gaps in the Disk Usage dashboard. Add a timeout and a short retry-with-backoff around those requests so a single dropped connection doesn't fail the whole run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add changelog entry for #25247 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use a requests Session with urllib3 Retry for wheel downloads Replaces the hand-rolled retry loop with a shared requests.Session whose HTTPAdapter is configured with a urllib3 Retry policy, letting the well-tested library handle backoff and retries instead of reimplementing them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Retry gateway errors and interrupted wheel downloads in ddev size The session retry only covered connection failures before the response headers, so a 504 from the wheels storage host or a connection dropped while streaming the wheel body still failed the disk-usage job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove verbose comments from wheel retry code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Limit wheel download restarts to the body read Connection and TLS errors before the response are already retried by the session, so restarting them in download_wheel multiplied attempts against an unreachable host. Also use download_wheel in ddev size timeline, and import Retry from requests instead of urllib3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Ignore Retry-After and test wheel retries against a real server urllib3 honors Retry-After on 413, 429 and 503 with no cap, which retried statuses outside the gateway list and could sleep for an hour. The retry tests now count the requests a local server receives instead of calling urllib3's Retry predicate directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Give the wheel body-restart loop its own limits download_wheel shared WHEEL_REQUEST_MAX_RETRIES and the backoff constant with the session retry, though the two layers compound and use different backoff formulas. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Restart interrupted wheel downloads with stamina Replaces the hand-written backoff loop in download_wheel, logs each restart through stamina's retry hook, and names the wheel in the error when every attempt fails. * Retry wheel requests with a single stamina attempt budget Replace the urllib3 adapter retries and the separate body-restart loop with one stamina retry around the whole request and body read, so a failure while connecting, a gateway error and a body cut short all count against the same four attempts instead of multiplying. The compressed timeline HEAD request goes through the same retry. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 4274300 commit 1635481

4 files changed

Lines changed: 182 additions & 12 deletions

File tree

‎ddev/changelog.d/25247.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Retry wheel downloads in `ddev size` on dropped connections and gateway errors, fixing recurring failures in the disk-usage measurement CI job.

‎ddev/src/ddev/cli/size/timeline.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
WrongDependencyFormat,
2424
compress,
2525
convert_to_human_readable_size,
26+
download_wheel,
2627
extract_version_from_about_py,
2728
get_valid_platforms,
2829
get_valid_versions,
2930
is_correct_dependency,
3031
is_valid_integration_file,
3132
print_table,
3233
request_wheel,
34+
retry_wheel_request,
3335
save_csv,
3436
save_json,
3537
save_markdown,
@@ -588,14 +590,15 @@ def get_dependency_size(
588590
A CommitEntry with size and metadata for the given dependency and commit.
589591
"""
590592
if compressed:
591-
response = request_wheel(app, download_url, wheels_storage, head=True)
593+
response = retry_wheel_request(
594+
lambda: request_wheel(app, download_url, wheels_storage, head=True), download_url
595+
)
592596
size_str = response.headers.get("Content-Length")
593597
if size_str is None:
594598
raise ValueError(f"Missing size for commit {commit}")
595599
size = int(size_str)
596600
else:
597-
with request_wheel(app, download_url, wheels_storage) as response:
598-
wheel_data = response.content
601+
wheel_data = download_wheel(app, download_url, wheels_storage)
599602

600603
with tempfile.TemporaryDirectory() as tmpdir:
601604
wheel_path = Path(tmpdir) / "package.whl"

‎ddev/src/ddev/cli/size/utils/common_funcs.py‎

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import tempfile
1313
import zipfile
1414
import zlib
15+
from collections.abc import Callable
1516
from datetime import date
1617
from types import TracebackType
1718
from typing import TYPE_CHECKING, Literal, Optional, Type, TypedDict, get_args
@@ -323,6 +324,13 @@ def wheel_url_candidates(url: str, wheels_storage: WheelsStorageTier) -> list[st
323324
return [resolve_wheel_url(url, tier) for tier in tiers]
324325

325326

327+
WHEEL_REQUEST_ATTEMPTS = 4
328+
WHEEL_REQUEST_RETRY_BACKOFF_SECONDS = 1.0
329+
WHEEL_REQUEST_TIMEOUT_SECONDS = 30
330+
331+
WHEEL_REQUEST_RETRY_STATUSES = (502, 503, 504)
332+
333+
326334
def request_wheel(
327335
app: Application, url: str, wheels_storage: WheelsStorageTier, head: bool = False
328336
) -> requests.Response:
@@ -331,14 +339,19 @@ def request_wheel(
331339
332340
Both 404 and 403 are treated as "not found" and trigger a fallback to the next tier, since the
333341
dev/stable S3 buckets return 403 instead of 404 for a missing key when the caller can't list the
334-
bucket. Any other error status aborts immediately instead of retrying. If every tier fails, the
335-
raised error lists all tried tiers, since the URL alone wouldn't reveal that a fallback was
336-
attempted.
342+
bucket. Any other error status aborts immediately instead of falling back. If every tier fails,
343+
the raised error lists all tried tiers, since the URL alone wouldn't reveal that a fallback was
344+
attempted. This function makes a single pass over the tiers and never retries; wrap it in
345+
`retry_wheel_request` to retry transient failures.
337346
"""
338347
candidates = wheel_url_candidates(url, wheels_storage)
339348
tried: list[str] = []
340349
for index, candidate in enumerate(candidates):
341-
response = requests.head(candidate) if head else requests.get(candidate, stream=True)
350+
response = (
351+
requests.head(candidate, timeout=WHEEL_REQUEST_TIMEOUT_SECONDS)
352+
if head
353+
else requests.get(candidate, stream=True, timeout=WHEEL_REQUEST_TIMEOUT_SECONDS)
354+
)
342355
try:
343356
response.raise_for_status()
344357
except requests.HTTPError as e:
@@ -355,6 +368,50 @@ def request_wheel(
355368
raise RuntimeError(f"No wheels storage tier served {url}")
356369

357370

371+
def is_transient_wheel_error(error: Exception) -> bool:
372+
"""
373+
Whether a failed wheel request or download is worth retrying: a dropped or timed-out connection,
374+
a body cut short mid-download, or a gateway error. Every other error status aborts.
375+
"""
376+
if isinstance(error, requests.HTTPError):
377+
return error.response is not None and error.response.status_code in WHEEL_REQUEST_RETRY_STATUSES
378+
return isinstance(error, (requests.ConnectionError, requests.Timeout, requests.exceptions.ChunkedEncodingError))
379+
380+
381+
def retry_wheel_request[T](operation: Callable[[], T], url: str) -> T:
382+
"""
383+
Runs a wheel request or download, retrying it on transient errors with exponential backoff.
384+
385+
A single attempt budget covers the whole operation, so a failure while connecting, a gateway
386+
error and a body cut short all count against the same `WHEEL_REQUEST_ATTEMPTS`.
387+
"""
388+
# Imported here because this module loads on every `ddev` run and `stamina` is only needed for wheels.
389+
import stamina
390+
391+
retrying = stamina.retry(
392+
on=is_transient_wheel_error,
393+
attempts=WHEEL_REQUEST_ATTEMPTS,
394+
timeout=None,
395+
wait_initial=WHEEL_REQUEST_RETRY_BACKOFF_SECONDS,
396+
)
397+
try:
398+
return retrying(operation)()
399+
except Exception as e:
400+
if is_transient_wheel_error(e):
401+
e.add_note(f"Wheel request for {url} failed after {WHEEL_REQUEST_ATTEMPTS} attempts")
402+
raise
403+
404+
405+
def download_wheel(app: Application, url: str, wheels_storage: WheelsStorageTier) -> bytes:
406+
"""Downloads a wheel's content, retrying the request or the body read on transient errors."""
407+
408+
def fetch() -> bytes:
409+
with request_wheel(app, url, wheels_storage) as response:
410+
return response.content
411+
412+
return retry_wheel_request(fetch, url)
413+
414+
358415
def get_dependencies(
359416
app: Application,
360417
repo_path: str | Path,
@@ -429,8 +486,7 @@ def get_dependencies_sizes(
429486
"""
430487
file_data: list[FileDataEntry] = []
431488
for dep, url, version in zip(deps, download_urls, versions, strict=False):
432-
with request_wheel(app, url, wheels_storage) as response:
433-
wheel_data = response.content
489+
wheel_data = download_wheel(app, url, wheels_storage)
434490

435491
with tempfile.TemporaryDirectory() as tmpdir:
436492
wheel_path = Path(tmpdir) / "package"

‎ddev/tests/size/test_common.py‎

Lines changed: 113 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,21 @@
33
import os
44
import re
55
import zipfile
6-
from unittest.mock import MagicMock, mock_open, patch
6+
from collections.abc import Iterator
7+
from unittest.mock import MagicMock, PropertyMock, mock_open, patch
78

89
import pytest
910
import requests
11+
import stamina
1012

1113
from ddev.cli.size.utils.common_funcs import (
14+
WHEEL_REQUEST_ATTEMPTS,
15+
WHEEL_REQUEST_TIMEOUT_SECONDS,
1216
_matches_gitignore,
1317
check_python_version,
1418
compress,
1519
convert_to_human_readable_size,
20+
download_wheel,
1621
export_format,
1722
extract_version_from_about_py,
1823
format_modules,
@@ -26,6 +31,7 @@
2631
is_correct_dependency,
2732
is_valid_integration_file,
2833
request_wheel,
34+
retry_wheel_request,
2935
save_csv,
3036
save_json,
3137
save_markdown,
@@ -191,7 +197,11 @@ def test_get_dependencies_sizes():
191197
)
192198

193199
# The storage tier placeholder must be resolved against the tier passed in, not left as-is.
194-
mock_get.assert_called_once_with("https://example.com/dev/dependency1/dependency1-1.1.1-.whl", stream=True)
200+
mock_get.assert_called_once_with(
201+
"https://example.com/dev/dependency1/dependency1-1.1.1-.whl",
202+
timeout=WHEEL_REQUEST_TIMEOUT_SECONDS,
203+
stream=True,
204+
)
195205

196206
assert file_data == [
197207
{
@@ -746,4 +756,104 @@ def test_request_wheel_uses_head_when_requested():
746756
with patch("requests.head", return_value=found) as mock_head:
747757
assert request_wheel(MagicMock(), PLACEHOLDER_URL, "stable", head=True) is found
748758

749-
mock_head.assert_called_once_with("https://example.com/stable/built/dep1/dep1-1.1.1-.whl")
759+
mock_head.assert_called_once_with(
760+
"https://example.com/stable/built/dep1/dep1-1.1.1-.whl", timeout=WHEEL_REQUEST_TIMEOUT_SECONDS
761+
)
762+
763+
764+
@pytest.fixture
765+
def instant_retries() -> Iterator[None]:
766+
"""Skips the backoff between wheel retries while keeping the configured number of attempts."""
767+
with stamina.set_testing(True, attempts=100, cap=True):
768+
yield
769+
770+
771+
def make_streamed_wheel_response(content):
772+
response = make_wheel_response(200)
773+
response.__enter__.return_value = response
774+
type(response).content = PropertyMock(side_effect=[content])
775+
return response
776+
777+
778+
@pytest.mark.parametrize(
779+
"make_failure",
780+
[
781+
pytest.param(lambda: make_wheel_response(502), id="bad gateway"),
782+
pytest.param(lambda: make_wheel_response(503), id="service unavailable"),
783+
pytest.param(lambda: make_wheel_response(504), id="gateway timeout"),
784+
pytest.param(lambda: requests.exceptions.ConnectionError("Connection reset"), id="connection reset"),
785+
pytest.param(lambda: requests.exceptions.ReadTimeout("Read timed out"), id="read timeout"),
786+
pytest.param(
787+
lambda: make_streamed_wheel_response(requests.exceptions.ChunkedEncodingError("Connection broken")),
788+
id="body cut short",
789+
),
790+
pytest.param(
791+
lambda: make_streamed_wheel_response(requests.exceptions.ConnectionError("Read timed out")),
792+
id="body read timeout",
793+
),
794+
],
795+
)
796+
def test_download_wheel_retries_transient_errors(instant_retries, make_failure):
797+
with patch("requests.get", side_effect=[make_failure(), make_streamed_wheel_response(b"wheel")]) as mock_get:
798+
assert download_wheel(MagicMock(), PLACEHOLDER_URL, "dev") == b"wheel"
799+
800+
assert mock_get.call_count == 2
801+
802+
803+
@pytest.mark.parametrize(
804+
"status_code, expected_requests",
805+
[
806+
pytest.param(500, 1, id="server error"),
807+
pytest.param(429, 1, id="too many requests"),
808+
pytest.param(404, 2, id="not found on every tier"),
809+
],
810+
)
811+
def test_download_wheel_does_not_retry_other_errors(instant_retries, status_code, expected_requests):
812+
responses = [make_wheel_response(status_code) for _ in range(WHEEL_REQUEST_ATTEMPTS)]
813+
814+
with patch("requests.get", side_effect=responses) as mock_get:
815+
with pytest.raises(requests.HTTPError, match=str(status_code)):
816+
download_wheel(MagicMock(), PLACEHOLDER_URL, "dev")
817+
818+
assert mock_get.call_count == expected_requests
819+
820+
821+
def make_failing_responses(kinds):
822+
return [
823+
make_wheel_response(504)
824+
if kind == "gateway error"
825+
else make_streamed_wheel_response(requests.exceptions.ChunkedEncodingError("Connection broken"))
826+
for kind in kinds
827+
]
828+
829+
830+
@pytest.mark.parametrize(
831+
"kinds",
832+
[
833+
pytest.param(["gateway error"] * (WHEEL_REQUEST_ATTEMPTS + 2), id="gateway errors"),
834+
pytest.param(["body cut short"] * (WHEEL_REQUEST_ATTEMPTS + 2), id="bodies cut short"),
835+
pytest.param(
836+
["gateway error"] * (WHEEL_REQUEST_ATTEMPTS - 1) + ["body cut short"] * 3,
837+
id="gateway errors then a body cut short",
838+
),
839+
],
840+
)
841+
def test_download_wheel_shares_one_attempt_budget_and_names_the_wheel(instant_retries, kinds):
842+
with patch("requests.get", side_effect=make_failing_responses(kinds)) as mock_get:
843+
with pytest.raises(requests.RequestException) as excinfo:
844+
download_wheel(MagicMock(), PLACEHOLDER_URL, "dev")
845+
846+
assert mock_get.call_count == WHEEL_REQUEST_ATTEMPTS
847+
assert any("dep1-1.1.1-.whl" in note for note in excinfo.value.__notes__)
848+
849+
850+
def test_retry_wheel_request_retries_a_head_request(instant_retries):
851+
found = make_wheel_response(200)
852+
853+
with patch("requests.head", side_effect=[make_wheel_response(504), found]) as mock_head:
854+
response = retry_wheel_request(
855+
lambda: request_wheel(MagicMock(), PLACEHOLDER_URL, "stable", head=True), PLACEHOLDER_URL
856+
)
857+
858+
assert response is found
859+
assert mock_head.call_count == 2

0 commit comments

Comments
 (0)