Use standard retry logic in _download_ranged

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `_download_ranged` no longer implements its own retry loop; transient failures are retried by the standard urllib3 retry configuration of the connection's session ([#934](https://github.com/Open-EO/openeo-python-client/issues/934))
 - Convert setup.py to pyproject.toml ([#920](https://github.com/Open-EO/openeo-python-client/issues/920))
 - Make `_DerivedFrom._from_url` more resilient against unresolvable/unparsable `derived_from` links ([#928](https://github.com/Open-EO/openeo-python-client/issues/928), eu-cdse/openeo-cdse-infra#1338)
 

openeo/rest/_connection.py

@@ -26,35 +26,14 @@
     str_truncate,
     url_join,
 )
-from openeo.utils.http import (
-    HTTP_408_REQUEST_TIMEOUT,
-    HTTP_429_TOO_MANY_REQUESTS,
-    HTTP_500_INTERNAL_SERVER_ERROR,
-    HTTP_501_NOT_IMPLEMENTED,
-    HTTP_502_BAD_GATEWAY,
-    HTTP_503_SERVICE_UNAVAILABLE,
-    HTTP_504_GATEWAY_TIMEOUT,
-    session_with_retries,
-)
+from openeo.utils.http import HTTP_502_BAD_GATEWAY, session_with_retries
 
 _log = logging.getLogger(__name__)
 
 # Default timeouts for requests
 # TODO: get default_timeout from config?
 DEFAULT_TIMEOUT = 20 * 60
 
-MAX_DOWNLOAD_RETRIES_PER_RANGE = 3
-
-RETRIABLE_DOWNLOAD_STATUSCODES = [
-    HTTP_408_REQUEST_TIMEOUT,
-    HTTP_429_TOO_MANY_REQUESTS,
-    HTTP_500_INTERNAL_SERVER_ERROR,
-    HTTP_501_NOT_IMPLEMENTED,
-    HTTP_502_BAD_GATEWAY,
-    HTTP_503_SERVICE_UNAVAILABLE,
-    HTTP_504_GATEWAY_TIMEOUT,
-]
-
 
 class RestApiConnection:
     """Base connection class implementing generic REST API request functionality"""
@@ -331,25 +310,15 @@ def _download_ranged(
         chunk_size: int = DEFAULT_DOWNLOAD_CHUNK_SIZE,
         range_size: int = DEFAULT_DOWNLOAD_RANGE_SIZE,
     ) -> None:
+        # Retries on transient failures (429/502/503/504) are handled by the
+        # urllib3 Retry mounted on this connection's session (see
+        # session_with_retries), so no per-range retry loop is needed here.
         ensure_parent_dir_for(target)
         with target.open("wb") as f:
             for from_byte_index in range(0, file_size, range_size):
                 to_byte_index = min(from_byte_index + range_size - 1, file_size - 1)
-                tries_left = MAX_DOWNLOAD_RETRIES_PER_RANGE
-                while tries_left > 0:
-                    try:
-                        range_headers = {"Range": f"bytes={from_byte_index}-{to_byte_index}"}
-                        with self.get(path=url, headers=range_headers, stream=True) as r:
-                            r.raise_for_status()
-                            for block in r.iter_content(chunk_size=chunk_size):
-                                f.write(block)
-                        break
-                    except OpenEoApiPlainError as error:
-                        tries_left -= 1
-                        if tries_left > 0 and error.http_status_code in RETRIABLE_DOWNLOAD_STATUSCODES:
-                            _log.warning(
-                                f"Failed to retrieve chunk {from_byte_index}-{to_byte_index} from {url} (status {error.http_status_code}) - retrying"
-                            )
-                            continue
-                        else:
-                            raise error
+                range_headers = {"Range": f"bytes={from_byte_index}-{to_byte_index}"}
+                with self.get(path=url, headers=range_headers, stream=True) as r:
+                    r.raise_for_status()
+                    for block in r.iter_content(chunk_size=chunk_size):
+                        f.write(block)

tests/rest/test_download_ranged_retry.py

@@ -0,0 +1,57 @@
+from pathlib import Path
+from unittest import mock
+
+import httpretty
+import pytest
+
+from openeo.rest._connection import RestApiConnection
+from openeo.utils.http import session_with_retries
+
+
+class TestDownloadRangedStandardRetry:
+    """
+    Regression test for #934: `_download_ranged` no longer implements its own
+    retry loop; transient failures are retried by the standard urllib3 Retry
+    configuration of the connection's session.
+    """
+
+    @pytest.fixture(autouse=True)
+    def _auto_httpretty_enabled(self):
+        with httpretty.enabled(allow_net_connect=False):
+            yield
+
+    def test_transient_503_on_range_is_retried_by_session(self, tmp_path, time_sleep):
+        content = b"0123456789" * 100  # 1000 bytes -> two 500-byte ranges
+        url = "https://example.test/dl/file.bin"
+
+        httpretty.register_uri(
+            httpretty.GET,
+            uri=url,
+            responses=[
+                # First attempt on range 0-499: transient failure.
+                httpretty.Response(status=503, body="Service Unavailable"),
+                # Retry of range 0-499 succeeds.
+                httpretty.Response(
+                    status=206,
+                    body=content[0:500],
+                    adding_headers={"Content-Range": f"bytes 0-499/{len(content)}"},
+                ),
+                # Range 500-999 succeeds on the first attempt.
+                httpretty.Response(
+                    status=206,
+                    body=content[500:],
+                    adding_headers={"Content-Range": f"bytes 500-999/{len(content)}"},
+                ),
+            ],
+        )
+
+        connection = RestApiConnection(root_url="https://example.test", session=session_with_retries())
+        target = tmp_path / "file.bin"
+        connection._download_ranged(url=url, target=target, file_size=len(content), range_size=500)
+
+        assert target.read_bytes() == content
+
+    @pytest.fixture
+    def time_sleep(self):
+        with mock.patch("time.sleep") as m:
+            yield m