Skip to content

Commit dd4443d

Browse files
committed
Retry requests rejected with HTTP status 429
Public Galaxy servers limit how many API requests a user may make, and reject the requests over that limit with HTTP status 429 (Too Many Requests). BioBlend did not handle those in any way: the request simply failed with a ConnectionError. Requests are now made through a requests session with a retrying adapter mounted, which honours the Retry-After header. All methods are retried, including POST ones: a 429 response means that the request was rejected before being processed, so replaying it cannot duplicate anything on the server. Retrying is bounded by the total time spent waiting, rather than by a number of attempts, so that a rate-limited call fails in a predictable amount of time instead of blocking for as long as the server asks for. urllib3 puts no upper bound on Retry-After, so both the individual waits and their total are capped, through the new `max_retry_after` and `max_total_retry_delay` properties. `max_429_retries` is a backstop for the case where the waits are all zero. Two kinds of requests are deliberately not retried: * multipart uploads, because neither urllib3 nor requests rewind the body between attempts, so a replayed request would send a truncated body and then block waiting on its own Content-Length; * requests failing with a connection or read error, which may have reached the server. These keep raising the same exceptions as before, since only the status counter of the retry policy is used. `Client._get()` does not retry a 429 response any more, as it has already been retried while honouring Retry-After; trying again would only add load to an overloaded server. Since a session is now created anyway, it can also be kept open to reuse connections across requests. This is opt-in through the new `use_session` property, as a shared session should not be used from multiple threads, while the per-request sessions used by default behave exactly like the `requests.get()` and friends used before.
1 parent 685c7f5 commit dd4443d

7 files changed

Lines changed: 684 additions & 45 deletions

File tree

CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,28 @@
1212
must be provided (thanks to
1313
[Dannon Baker](https://github.com/dannon)).
1414

15+
* Requests rejected with HTTP status 429 (Too Many Requests) are now retried
16+
automatically, honouring the ``Retry-After`` header. All HTTP methods are
17+
retried, since a 429 response means that the request was rejected before being
18+
processed. Multipart uploads are not retried, because their body cannot be
19+
replayed.
20+
21+
* Added ``max_total_retry_delay``, ``max_retry_after`` and ``max_429_retries``
22+
properties to ``GalaxyClient``, bounding the above. Retrying stops after a
23+
total of 60 s of waiting by default; set ``max_total_retry_delay`` to 0 to
24+
disable it.
25+
26+
* Added ``use_session`` property, ``close()`` method and context manager support
27+
to ``GalaxyClient``, to reuse connections across requests. Disabled by
28+
default.
29+
30+
* ``Client._get()`` does not retry requests rejected with HTTP status 429 any
31+
more, as they have already been retried. This only affects users who set
32+
``max_get_attempts`` to a value greater than 1.
33+
34+
* Added dependency on ``urllib3`` >= 2.0, previously an indirect one through
35+
``requests``.
36+
1537
## BioBlend v1.9.0 - 2026-04-14
1638

1739
* Added ``create_credentials()``, ``get_credentials()``,
Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
"""
2+
Tests on the handling of HTTP status 429 (Too Many Requests) responses.
3+
4+
These tests do not need a Galaxy instance: they run against a minimal HTTP
5+
server replying with a canned sequence of status codes. A real server is needed
6+
because the retrying happens inside urllib3, below the layer at which HTTP
7+
mocking libraries usually work.
8+
"""
9+
10+
import threading
11+
import time
12+
import unittest
13+
from http.server import (
14+
BaseHTTPRequestHandler,
15+
ThreadingHTTPServer,
16+
)
17+
from typing import Any
18+
19+
import pytest
20+
21+
from bioblend import ConnectionError
22+
from bioblend.galaxy import GalaxyInstance
23+
24+
25+
class MockServer:
26+
"""
27+
HTTP server replying with a canned sequence of status codes.
28+
29+
The last status code of the sequence is repeated for any further request.
30+
"""
31+
32+
def __init__(self, statuses: list[int], retry_after: int | None = None) -> None:
33+
self.statuses = statuses
34+
self.retry_after = retry_after
35+
self.requests: list[tuple[str, str]] = []
36+
lock = threading.Lock()
37+
server = self
38+
39+
class Handler(BaseHTTPRequestHandler):
40+
def _reply(self) -> None:
41+
# The request body must be consumed, otherwise the client may
42+
# block while writing it.
43+
content_length = int(self.headers.get("Content-Length") or 0)
44+
if content_length:
45+
self.rfile.read(content_length)
46+
with lock:
47+
index = len(server.requests)
48+
server.requests.append((self.command, self.path))
49+
status = server.statuses[min(index, len(server.statuses) - 1)]
50+
body = b'{"ok": true}' if status == 200 else b'{"error": "nope"}'
51+
self.send_response(status)
52+
if status == 429 and server.retry_after is not None:
53+
self.send_header("Retry-After", str(server.retry_after))
54+
self.send_header("Content-Type", "application/json")
55+
self.send_header("Content-Length", str(len(body)))
56+
self.end_headers()
57+
self.wfile.write(body)
58+
59+
do_GET = _reply
60+
do_POST = _reply
61+
do_PUT = _reply
62+
do_PATCH = _reply
63+
do_DELETE = _reply
64+
65+
def log_message(self, format: str, *args: Any) -> None:
66+
pass
67+
68+
self._httpd = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
69+
self.url = f"http://127.0.0.1:{self._httpd.server_address[1]}"
70+
71+
@property
72+
def request_count(self) -> int:
73+
return len(self.requests)
74+
75+
def __enter__(self) -> "MockServer":
76+
self._thread = threading.Thread(target=self._httpd.serve_forever, daemon=True)
77+
self._thread.start()
78+
return self
79+
80+
def __exit__(self, *args: object) -> None:
81+
self._httpd.shutdown()
82+
self._httpd.server_close()
83+
self._thread.join()
84+
85+
86+
def galaxy_instance(server: MockServer, **kwargs: object) -> GalaxyInstance:
87+
"""
88+
Return a ``GalaxyInstance`` pointing at ``server``, with retry delays short
89+
enough to keep the tests fast.
90+
"""
91+
gi = GalaxyInstance(server.url, key="whatever")
92+
gi.max_retry_after = 0.1
93+
gi.max_total_retry_delay = 1.0
94+
for name, value in kwargs.items():
95+
setattr(gi, name, value)
96+
return gi
97+
98+
99+
class TestGalaxyRateLimit(unittest.TestCase):
100+
def test_get_is_retried(self):
101+
with MockServer([429, 429, 200], retry_after=1) as server:
102+
gi = galaxy_instance(server)
103+
r = gi.make_get_request(f"{gi.url}/libraries")
104+
assert r.status_code == 200
105+
assert server.request_count == 3
106+
107+
def test_post_is_retried(self):
108+
# A 429 response means the request was rejected before being processed,
109+
# so even a non-idempotent method can safely be replayed.
110+
with MockServer([429, 429, 200], retry_after=1) as server:
111+
gi = galaxy_instance(server)
112+
assert gi.make_post_request(f"{gi.url}/histories", payload={"name": "test"}) == {"ok": True}
113+
assert server.request_count == 3
114+
assert [method for method, _ in server.requests] == ["POST"] * 3
115+
116+
def test_put_and_delete_are_retried(self):
117+
with MockServer([429, 200], retry_after=1) as server:
118+
gi = galaxy_instance(server)
119+
assert gi.make_put_request(f"{gi.url}/histories/abc", payload={"name": "test"}) == {"ok": True}
120+
assert server.request_count == 2
121+
with MockServer([429, 200], retry_after=1) as server:
122+
gi = galaxy_instance(server)
123+
r = gi.make_delete_request(f"{gi.url}/histories/abc")
124+
assert r.status_code == 200
125+
assert server.request_count == 2
126+
127+
def test_multipart_post_is_not_retried(self):
128+
# The body of a multipart request is a stream which is not rewound
129+
# between attempts, so replaying it would send a truncated body.
130+
with MockServer([429, 200], retry_after=1) as server:
131+
gi = galaxy_instance(server)
132+
with pytest.raises(ConnectionError) as excinfo:
133+
gi.make_post_request(f"{gi.url}/tools", payload={"name": "test"}, files_attached=True)
134+
assert excinfo.value.status_code == 429
135+
assert server.request_count == 1
136+
137+
def test_retries_exhausted_raises_connection_error(self):
138+
with MockServer([429], retry_after=1) as server:
139+
gi = galaxy_instance(server)
140+
with pytest.raises(ConnectionError) as excinfo:
141+
gi.make_post_request(f"{gi.url}/histories", payload={})
142+
assert excinfo.value.status_code == 429
143+
assert excinfo.value.body == '{"error": "nope"}'
144+
assert server.request_count > 1
145+
146+
def test_max_total_retry_delay_caps_the_wait(self):
147+
# The server asks to wait much longer than the budget allows.
148+
with MockServer([429], retry_after=3600) as server:
149+
gi = galaxy_instance(server, max_retry_after=0.2, max_total_retry_delay=0.5)
150+
start = time.monotonic()
151+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 429
152+
duration = time.monotonic() - start
153+
# Worst case is max_total_retry_delay + max_retry_after, plus the
154+
# time spent on the requests themselves.
155+
assert duration < 3, f"Retrying took {duration} s, ignoring the delay budget"
156+
157+
def test_max_total_retry_delay_of_zero_disables_retrying(self):
158+
with MockServer([429, 200], retry_after=1) as server:
159+
gi = galaxy_instance(server, max_total_retry_delay=0)
160+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 429
161+
assert server.request_count == 1
162+
163+
def test_max_429_retries_limits_retrying_without_delays(self):
164+
# With no delay between attempts the budget is never consumed, so the
165+
# number of retries is what stops the loop.
166+
with MockServer([429], retry_after=0) as server:
167+
gi = galaxy_instance(server, max_retry_after=0, max_total_retry_delay=60, max_429_retries=3)
168+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 429
169+
assert server.request_count == 4
170+
171+
def test_other_error_statuses_are_not_retried(self):
172+
with MockServer([500]) as server:
173+
gi = galaxy_instance(server)
174+
with pytest.raises(ConnectionError) as excinfo:
175+
gi.make_post_request(f"{gi.url}/histories", payload={})
176+
assert excinfo.value.status_code == 500
177+
assert server.request_count == 1
178+
179+
def test_get_client_does_not_retry_429_again(self):
180+
# `_get()` retries failed GET requests, but a 429 response has already
181+
# been retried by the session, so it must not be tried again.
182+
with MockServer([429], retry_after=0) as server:
183+
gi = galaxy_instance(
184+
server,
185+
max_retry_after=0,
186+
max_429_retries=0,
187+
max_get_attempts=3,
188+
get_retry_delay=30,
189+
)
190+
start = time.monotonic()
191+
with pytest.raises(ConnectionError) as excinfo:
192+
gi.libraries.get_libraries()
193+
duration = time.monotonic() - start
194+
assert excinfo.value.status_code == 429
195+
assert server.request_count == 1
196+
assert duration < 5, f"Took {duration} s, the GET retry loop was not skipped"
197+
198+
def test_get_client_still_retries_other_errors(self):
199+
with MockServer([500, 500, 200]) as server:
200+
gi = galaxy_instance(server, max_get_attempts=3, get_retry_delay=0)
201+
libraries: Any = gi.libraries.get_libraries()
202+
assert libraries == {"ok": True}
203+
assert server.request_count == 3
204+
205+
def test_streamed_response_is_readable(self):
206+
with MockServer([429, 200], retry_after=1) as server:
207+
gi = galaxy_instance(server)
208+
r = gi.make_get_request(f"{gi.url}/datasets/abc/display", stream=True)
209+
assert r.status_code == 200
210+
assert b"".join(r.iter_content(4)) == b'{"ok": true}'
211+
212+
213+
class TestGalaxySession(unittest.TestCase):
214+
def test_session_is_not_used_by_default(self):
215+
with MockServer([200]) as server:
216+
gi = galaxy_instance(server)
217+
assert gi.use_session is False
218+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 200
219+
220+
def test_session_is_used_and_retries_when_enabled(self):
221+
with MockServer([429, 200], retry_after=1) as server:
222+
gi = galaxy_instance(server, use_session=True)
223+
assert gi.use_session is True
224+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 200
225+
assert server.request_count == 2
226+
227+
def test_requests_keep_working_after_closing_the_session(self):
228+
with MockServer([200]) as server:
229+
gi = galaxy_instance(server, use_session=True)
230+
gi.close()
231+
assert gi.use_session is False
232+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 200
233+
234+
def test_context_manager_enables_and_closes_the_session(self):
235+
with MockServer([200]) as server:
236+
gi = galaxy_instance(server)
237+
with gi as entered:
238+
assert entered is gi
239+
used_inside = gi.use_session
240+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 200
241+
assert used_inside is True
242+
assert gi.use_session is False
243+
244+
def test_changing_settings_keeps_the_session_usable(self):
245+
with MockServer([429, 200], retry_after=1) as server:
246+
gi = galaxy_instance(server, use_session=True)
247+
gi.max_429_retries = 5
248+
assert gi.use_session is True
249+
assert gi.make_get_request(f"{gi.url}/libraries").status_code == 200
250+
251+
252+
class TestGalaxyRetrySettings(unittest.TestCase):
253+
def setUp(self):
254+
self.gi = GalaxyInstance("http://localhost:56789", key="whatever")
255+
256+
def test_defaults(self):
257+
assert self.gi.max_429_retries == 10
258+
assert self.gi.max_retry_after == 30.0
259+
assert self.gi.max_total_retry_delay == 60.0
260+
assert self.gi.use_session is False
261+
262+
def test_settings_can_be_changed(self):
263+
self.gi.max_429_retries = 2
264+
assert self.gi.max_429_retries == 2
265+
self.gi.max_retry_after = 1.5
266+
assert self.gi.max_retry_after == 1.5
267+
self.gi.max_total_retry_delay = 5.0
268+
assert self.gi.max_total_retry_delay == 5.0
269+
270+
def test_negative_settings_are_rejected(self):
271+
with pytest.raises(ValueError):
272+
self.gi.max_429_retries = -1
273+
with pytest.raises(ValueError):
274+
self.gi.max_retry_after = -1.0
275+
with pytest.raises(ValueError):
276+
self.gi.max_total_retry_delay = -1.0

bioblend/galaxy/client.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,12 @@ def _get(
155155
msg = f"GET: invalid JSON : {r.content!r}"
156156
else:
157157
msg = f"GET: error {r.status_code}: {r.content!r}"
158+
if r.status_code == 429:
159+
# Rate-limited requests are already retried while
160+
# honouring the Retry-After header, see gi's
161+
# `max_total_retry_delay` property. Trying again here
162+
# would only add load to an overloaded server.
163+
attempts_left = 0
158164
msg = f"{msg}, {attempts_left} attempts left"
159165
if attempts_left <= 0:
160166
bioblend.log.error(msg)

0 commit comments

Comments
 (0)