Skip to content

feat(retry) add optional randomness min/max #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(retry) add optional randomness min/max #50

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/dicomweb_client/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ def set_http_retry_params(
retry: bool = True,
max_attempts: int = 5,
wait_exponential_multiplier: int = 1000,
wait_random_min: int = None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
wait_random_min: int = None,
wait_random_min: Optional[int] = None,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the random from the parameter names and just use wait_min and wait_max rather than wait_random_min and wait_random_max, respectively?

wait_random_max: int = None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
wait_random_max: int = None,
wait_random_max: Optional[int] = None,

retriable_error_codes: Tuple[HTTPStatus, ...] = (
HTTPStatus.TOO_MANY_REQUESTS,
HTTPStatus.REQUEST_TIMEOUT,
Expand All @@ -153,7 +155,8 @@ def set_http_retry_params(
responses that return an error code defined in |retriable_error_codes|.
The retrying method uses exponential back off using the multiplier
|wait_exponential_multiplier| for a max attempts defined by
|max_attempts|.
|max_attempts| with randomness defined by |wait_random_min] and
|wait_random_max] in ms (None by default for no randomness).

Parameters
----------
Expand All @@ -164,6 +167,10 @@ def set_http_retry_params(
the maximum number of request attempts.
wait_exponential_multiplier: float, optional
exponential multiplier applied to delay between attempts in ms.
wait_random_min: float, optional
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
wait_random_min: float, optional
wait_random_min: int, optional

minimum amount of jitter in ms.
wait_random_max: float, optional
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
wait_random_max: float, optional
wait_random_max: int, optional

maximum amount of jitter in ms.
retriable_error_codes: tuple, optional
tuple of HTTP error codes to retry if raised.

Expand All @@ -172,6 +179,8 @@ def set_http_retry_params(
if retry:
self._max_attempts = max_attempts
self._wait_exponential_multiplier = wait_exponential_multiplier
self._wait_random_min = wait_random_min
self._wait_random_max = wait_random_max
self._http_retrable_errors = retriable_error_codes

else:
Expand Down Expand Up @@ -589,6 +598,8 @@ def _http_get(
@retrying.retry(
retry_on_result=self._is_retriable_http_error,
wait_exponential_multiplier=self._wait_exponential_multiplier,
wait_random_min=self._wait_random_min,
wait_random_max=self._wait_random_max,
stop_max_attempt_number=self._max_attempts
)
def _invoke_get_request(
Expand Down Expand Up @@ -1493,6 +1504,8 @@ def serve_data_chunks(data):
@retrying.retry(
retry_on_result=self._is_retriable_http_error,
wait_exponential_multiplier=self._wait_exponential_multiplier,
wait_random_min=self._wait_random_min,
wait_random_max=self._wait_random_max,
stop_max_attempt_number=self._max_attempts
)
def _invoke_post_request(
Expand Down Expand Up @@ -1599,6 +1612,8 @@ def _http_delete(self, url: str):
@retrying.retry(
retry_on_result=self._is_retriable_http_error,
wait_exponential_multiplier=self._wait_exponential_multiplier,
wait_random_min=self._wait_random_min,
wait_random_max=self._wait_random_max,
stop_max_attempt_number=self._max_attempts
)
def _invoke_delete_request(url: str) -> requests.models.Response:
Expand Down