Skip to content

Commit 651f953

Browse files
authored
Merge pull request #22567 from nsoranzo/auto_requests_retries
Add ``RetrySession`` to automatically retry idempotent HTTP requests
2 parents 4ffbe92 + 6060bad commit 651f953

6 files changed

Lines changed: 147 additions & 35 deletions

File tree

lib/galaxy/tool_util/deps/mulled/mulled_build_channel.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def _new_versions(quay, conda):
6565

6666
def run_channel(args, build_last_n_versions: int = 1) -> None:
6767
"""Build list of involucro commands (as shell snippet) to run."""
68-
session = requests.session()
68+
session = requests.Session()
6969
for pkg_name, pkg_tests in get_affected_packages(args):
7070
repo_data = _fetch_repo_data(args)
7171
c = conda_versions(pkg_name, repo_data)

lib/galaxy/tool_util/deps/mulled/util.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ def quay_repository(namespace: str, pkg_name: str, session: Optional[Session] =
123123
assert pkg_name is not None
124124
url = f"https://quay.io/api/v1/repository/{namespace}/{pkg_name}"
125125
if not session:
126-
session = requests.session()
126+
session = requests.Session()
127127
response = session.get(url, timeout=MULLED_SOCKET_TIMEOUT)
128128
data = response.json()
129129
return data

lib/galaxy/util/__init__.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@
6262
default_enter,
6363
remap,
6464
)
65-
from requests.adapters import HTTPAdapter
66-
from requests.packages.urllib3.util.retry import Retry # type: ignore[import-untyped, unused-ignore]
6765
from typing_extensions import (
6866
Literal,
6967
Self,
@@ -1945,10 +1943,8 @@ def build_url(base_url, port=80, scheme="http", pathspec=None, params=None, dose
19451943
def url_get(base_url, auth=None, pathspec=None, params=None, max_retries=5, backoff_factor=1):
19461944
"""Make contact with the uri provided and return any contents."""
19471945
full_url = build_url(base_url, pathspec=pathspec, params=params)
1948-
s = requests.Session()
1949-
retries = Retry(total=max_retries, backoff_factor=backoff_factor, status_forcelist=[429])
1950-
s.mount(base_url, HTTPAdapter(max_retries=retries))
1951-
response = s.get(full_url, auth=auth)
1946+
with requests.RetrySession(total=max_retries, backoff_factor=backoff_factor, status_forcelist=[429]) as s:
1947+
response = s.get(full_url, auth=auth)
19521948
response.raise_for_status()
19531949
return response.text
19541950

lib/galaxy/util/requests.py

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from typing import (
22
Callable,
3-
cast,
4-
TypeVar,
3+
Union,
54
)
65

76
import requests
@@ -10,36 +9,49 @@
109
exceptions as exceptions,
1110
Response as Response,
1211
)
12+
from requests.adapters import HTTPAdapter
13+
from requests.packages.urllib3.util.retry import Retry
1314
from typing_extensions import ParamSpec
1415

1516
from .user_agent import get_default_headers
1617

17-
Param = ParamSpec("Param")
18-
RetType = TypeVar("RetType")
18+
DEFAULT_RETRIES = 3
19+
DEFAULT_BACKOFF_FACTOR = 0.1
20+
21+
22+
class Session(requests.Session):
23+
def __init__(self) -> None:
24+
super().__init__()
25+
self.headers.update(get_default_headers())
26+
1927

28+
class RetrySession(Session):
29+
def __init__(
30+
self, total: Union[bool, int, None] = DEFAULT_RETRIES, backoff_factor: float = DEFAULT_BACKOFF_FACTOR, **kwargs
31+
) -> None:
32+
super().__init__()
33+
retry = Retry(total=total, backoff_factor=backoff_factor, **kwargs)
34+
adapter = HTTPAdapter(max_retries=retry)
35+
self.mount("https://", adapter)
36+
self.mount("http://", adapter)
37+
38+
39+
Param = ParamSpec("Param")
2040

21-
def default_user_agent_decorator(f: Callable[Param, RetType]) -> Callable[Param, RetType]:
2241

23-
def wrapper(*args: Param.args, **kwargs: Param.kwargs) -> RetType:
24-
headers = cast(dict, kwargs.pop("headers", None) or {})
25-
headers.update(get_default_headers())
26-
is_session = f in (requests.session, requests.Session)
27-
if not is_session:
28-
kwargs["headers"] = headers
29-
rval = f(*args, **kwargs)
30-
if is_session:
31-
rval.headers = headers # type: ignore[attr-defined]
32-
return rval
42+
def _request_decorator(f: Callable[Param, Response]) -> Callable[Param, Response]:
43+
def wrapper(*args: Param.args, **kwargs: Param.kwargs) -> Response:
44+
with Session() as s:
45+
return getattr(s, f.__name__)(*args, **kwargs)
3346

3447
return wrapper
3548

3649

37-
delete = default_user_agent_decorator(requests.delete)
38-
get = default_user_agent_decorator(requests.get)
39-
head = default_user_agent_decorator(requests.head)
40-
patch = default_user_agent_decorator(requests.patch)
41-
post = default_user_agent_decorator(requests.post)
42-
options = default_user_agent_decorator(requests.options)
43-
put = default_user_agent_decorator(requests.put)
44-
session = default_user_agent_decorator(requests.session)
45-
Session = default_user_agent_decorator(requests.Session)
50+
delete = _request_decorator(requests.delete)
51+
get = _request_decorator(requests.get)
52+
head = _request_decorator(requests.head)
53+
patch = _request_decorator(requests.patch)
54+
post = _request_decorator(requests.post)
55+
options = _request_decorator(requests.options)
56+
put = _request_decorator(requests.put)
57+
session = Session

lib/galaxy/webapps/galaxy/api/container_resolution.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def resolve(self, trans, index=None, **kwds):
6464
:returns: a dictified description of the container dependency, with attribute
6565
``dependency_type: None`` if no match was found.
6666
"""
67-
kwds["session"] = requests.session()
67+
kwds["session"] = requests.Session()
6868
return self._view.resolve(index=index, **kwds)
6969

7070
@expose_api
@@ -84,7 +84,7 @@ def resolve_toolbox(self, trans, **kwds):
8484
:rtype: list
8585
:returns: list of items returned from resolve()
8686
"""
87-
kwds["session"] = requests.session()
87+
kwds["session"] = requests.Session()
8888
return self._view.resolve_toolbox(**kwds)
8989

9090
@expose_api
@@ -103,7 +103,7 @@ def resolve_toolbox_with_install(self, trans, payload, **kwds):
103103
"""
104104
kwds.update(payload)
105105
kwds["install"] = True
106-
kwds["session"] = requests.session()
106+
kwds["session"] = requests.Session()
107107
return self._view.resolve_toolbox(**kwds)
108108

109109
@expose_api

test/unit/util/test_requests.py

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import socket
2+
import threading
3+
4+
import pytest
5+
import responses
6+
from requests.adapters import HTTPAdapter
7+
8+
from galaxy.util import requests as galaxy_requests
9+
from galaxy.util.requests import DEFAULT_RETRIES
10+
from galaxy.util.user_agent import get_default_headers
11+
12+
EXPECTED_USER_AGENT = get_default_headers()["user-agent"]
13+
14+
15+
# --- User-Agent header injection ---
16+
17+
18+
@pytest.mark.parametrize("method", ("delete", "get", "head", "options", "patch", "post", "put"))
19+
@responses.activate
20+
def test_user_agent_and_caller_headers_are_set(method: str):
21+
"""All methods inject the Galaxy user-agent and preserve caller-supplied headers."""
22+
responses.add(getattr(responses, method.upper()), "https://example.com/", status=200)
23+
getattr(galaxy_requests, method)("https://example.com/", headers={"X-Custom": "value"})
24+
req_headers = responses.calls[0].request.headers
25+
assert req_headers["user-agent"] == EXPECTED_USER_AGENT
26+
assert req_headers["X-Custom"] == "value"
27+
28+
29+
# --- Session factory ---
30+
31+
32+
@pytest.mark.parametrize("method", (galaxy_requests.Session, galaxy_requests.session))
33+
def test_Session_has_user_agent_header(method):
34+
s: galaxy_requests.Session = method()
35+
assert s.headers["user-agent"] == EXPECTED_USER_AGENT
36+
37+
38+
def test_Session_has_no_retry_adapter():
39+
s = galaxy_requests.Session()
40+
for url in ("https://example.com/", "http://example.com/"):
41+
adapter = s.get_adapter(url)
42+
assert isinstance(adapter, HTTPAdapter)
43+
assert adapter.max_retries.total == 0 # requests default: no retries
44+
45+
46+
# --- RetrySession ---
47+
48+
49+
def test_RetrySession_has_user_agent_header():
50+
s = galaxy_requests.RetrySession()
51+
assert s.headers["user-agent"] == EXPECTED_USER_AGENT
52+
53+
54+
def test_retry_session_has_retry_adapter():
55+
s = galaxy_requests.RetrySession()
56+
for url in ("https://example.com/", "http://example.com/"):
57+
adapter = s.get_adapter(url)
58+
assert isinstance(adapter, HTTPAdapter)
59+
assert adapter.max_retries.total == DEFAULT_RETRIES
60+
61+
62+
# --- Retry behaviour ---
63+
64+
65+
@pytest.fixture()
66+
def connection_reset_server():
67+
"""A TCP server that accepts connections and immediately closes them."""
68+
attempts = 0
69+
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
70+
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
71+
sock.bind(("127.0.0.1", 0))
72+
port = sock.getsockname()[1]
73+
sock.listen(10)
74+
stop = threading.Event()
75+
sock.settimeout(0.1)
76+
77+
def serve():
78+
nonlocal attempts
79+
while not stop.is_set():
80+
try:
81+
conn, _ = sock.accept()
82+
except socket.timeout: # noqa: UP041 # Python <=3.9 support, replace with TimeoutError in Python 3.10+
83+
continue
84+
except OSError:
85+
break
86+
attempts += 1
87+
conn.close()
88+
89+
thread = threading.Thread(target=serve, daemon=True)
90+
thread.start()
91+
yield f"http://127.0.0.1:{port}", lambda: attempts
92+
stop.set()
93+
sock.close()
94+
thread.join()
95+
96+
97+
@pytest.mark.parametrize("method", ("delete", "get", "head", "options", "put"))
98+
def test_RetrySession_retries_idempotent_methods(method: str, connection_reset_server):
99+
"""RetrySession retries idempotent methods on connection failure."""
100+
url, get_attempts = connection_reset_server
101+
with galaxy_requests.RetrySession() as s:
102+
with pytest.raises(galaxy_requests.exceptions.ConnectionError):
103+
getattr(s, method)(url)
104+
assert get_attempts() == 1 + DEFAULT_RETRIES

0 commit comments

Comments
 (0)