Retry requests rejected with HTTP status 429 - #542
Open
mvdbeek wants to merge 2 commits into
Open
Conversation
nsoranzo
reviewed
Jul 29, 2026
Comment on lines
+271
to
+288
| @property | ||
| def use_session(self) -> bool: | ||
| """ | ||
| Whether a single session is reused for all requests. Default: ``False`` | ||
|
|
||
| Enabling this reuses connections across requests, which is faster when | ||
| making many of them. The resulting object should not be shared between | ||
| threads, and ``close()`` should be called when done with it. | ||
| """ | ||
| return self._session is not None | ||
|
|
||
| @use_session.setter | ||
| def use_session(self, value: bool) -> None: | ||
| if value: | ||
| if self._session is None: | ||
| self._session = self._new_session() | ||
| else: | ||
| self.close() |
Member
There was a problem hiding this comment.
I think I'd prefer to have use_session just be an boolean instance attribute, and _session_ctx() would store a newly created session only if use_session is True.
Or even, is there a use case for setting use_session to False? Should we just always store a session?
Member
Author
There was a problem hiding this comment.
Threads may share a session and that isn't safe. I think we'd want to explicitly delineate what goes into a session? Hence the context manager.
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.
The example made a single request, which is exactly the case where reusing a connection makes no difference. Make one request per history returned, and quote strings with `"` as in the rest of the documentation. Writing the example also showed that `__enter__()` was annotated as returning `GalaxyClient`, so the subclass was lost and accessing e.g. `gi.histories` on the result did not type check. Return `Self` instead, as done elsewhere in the package.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Public Galaxy servers limit how many API requests a user may make in a given period of time, and reject the requests made 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
requestssession with a retrying adapter mounted, which honours theRetry-Afterheader.What is retried
All methods, including
POST: a 429 response means that the request was rejected before being processed, so replaying it cannot duplicate anything on the server.Two kinds of requests are deliberately not retried:
Content-Length.make_post_request(files_attached=True)therefore uses a non-retrying session.How 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 honours
Retry-Afterbut puts no upper bound on it, so both the individual waits and their total are capped:max_total_retry_delaymax_retry_afterRetry-Aftermax_429_retriesOnce the budget is spent, the usual
ConnectionErroris raised with the 429 status code and response body, so the existing contract is unchanged. Worst case ismax_total_retry_delay + max_retry_after, as the budget is checked after each wait.Client._get()no longer retries a 429 response, since it has already been retried while honouringRetry-After; trying again would only add load to an overloaded server. This only affects users who setmax_get_attemptsto a value greater than 1.Connection reuse
Since a session is created anyway, it can also be kept open to reuse connections across requests. This is opt-in through the new
use_sessionproperty (plusclose()and context manager support), because a shared session should not be used from multiple threads. By default a session is created and closed per request, which is exactly whatrequests.get()and friends did before, so there is no change for existing scripts.Tests
bioblend/_tests/TestGalaxyRateLimit.pyadds 20 tests which do not need a Galaxy instance. They run against a minimalhttp.server-based server rather than using an HTTP mocking library, because the retrying happens inside urllib3, below the layer at which those libraries replaceHTTPAdapter.send— mocking would silently bypass the whole feature.Note
Uploads through the tus endpoint (
get_tus_uploader()) are not covered, as they are handled bytusclient, which has its own retry settings. Happy to wire that up too if wanted.