Skip to content

Commit

Permalink
fix: Retry constructors methods support None (#592)
Browse files Browse the repository at this point in the history
  • Loading branch information
daniel-sanche authored Feb 5, 2024
1 parent b517bf4 commit 416203c
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 49 deletions.
77 changes: 42 additions & 35 deletions google/api_core/retry/retry_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import time

from enum import Enum
from typing import Any, Callable, TYPE_CHECKING
from typing import Any, Callable, Optional, TYPE_CHECKING

import requests.exceptions

Expand Down Expand Up @@ -238,8 +238,8 @@ def __init__(
initial: float = _DEFAULT_INITIAL_DELAY,
maximum: float = _DEFAULT_MAXIMUM_DELAY,
multiplier: float = _DEFAULT_DELAY_MULTIPLIER,
timeout: float = _DEFAULT_DEADLINE,
on_error: Callable[[Exception], Any] | None = None,
timeout: Optional[float] = _DEFAULT_DEADLINE,
on_error: Optional[Callable[[Exception], Any]] = None,
**kwargs: Any,
) -> None:
self._predicate = predicate
Expand All @@ -265,48 +265,39 @@ def deadline(self) -> float | None:
def timeout(self) -> float | None:
return self._timeout

def _replace(
self,
predicate: Callable[[Exception], bool] | None = None,
initial: float | None = None,
maximum: float | None = None,
multiplier: float | None = None,
timeout: float | None = None,
on_error: Callable[[Exception], Any] | None = None,
) -> Self:
return type(self)(
predicate=predicate or self._predicate,
initial=initial or self._initial,
maximum=maximum or self._maximum,
multiplier=multiplier or self._multiplier,
timeout=timeout or self._timeout,
on_error=on_error or self._on_error,
)

def with_deadline(self, deadline: float | None) -> Self:
"""Return a copy of this retry with the given timeout.
DEPRECATED: use :meth:`with_timeout` instead. Refer to the ``Retry`` class
documentation for details.
Args:
deadline (float): How long to keep retrying, in seconds.
deadline (float|None): How long to keep retrying, in seconds. If None,
no timeout is enforced.
Returns:
Retry: A new retry instance with the given timeout.
"""
return self._replace(timeout=deadline)
return self.with_timeout(deadline)

def with_timeout(self, timeout: float) -> Self:
def with_timeout(self, timeout: float | None) -> Self:
"""Return a copy of this retry with the given timeout.
Args:
timeout (float): How long to keep retrying, in seconds.
timeout (float): How long to keep retrying, in seconds. If None,
no timeout will be enforced.
Returns:
Retry: A new retry instance with the given timeout.
"""
return self._replace(timeout=timeout)
return type(self)(
predicate=self._predicate,
initial=self._initial,
maximum=self._maximum,
multiplier=self._multiplier,
timeout=timeout,
on_error=self._on_error,
)

def with_predicate(self, predicate: Callable[[Exception], bool]) -> Self:
"""Return a copy of this retry with the given predicate.
Expand All @@ -318,26 +309,42 @@ def with_predicate(self, predicate: Callable[[Exception], bool]) -> Self:
Returns:
Retry: A new retry instance with the given predicate.
"""
return self._replace(predicate=predicate)
return type(self)(
predicate=predicate,
initial=self._initial,
maximum=self._maximum,
multiplier=self._multiplier,
timeout=self._timeout,
on_error=self._on_error,
)

def with_delay(
self,
initial: float | None = None,
maximum: float | None = None,
multiplier: float | None = None,
initial: Optional[float] = None,
maximum: Optional[float] = None,
multiplier: Optional[float] = None,
) -> Self:
"""Return a copy of this retry with the given delay options.
Args:
initial (float): The minimum amount of time to delay (in seconds). This must
be greater than 0.
maximum (float): The maximum amount of time to delay (in seconds).
multiplier (float): The multiplier applied to the delay.
be greater than 0. If None, the current value is used.
maximum (float): The maximum amount of time to delay (in seconds). If None, the
current value is used.
multiplier (float): The multiplier applied to the delay. If None, the current
value is used.
Returns:
Retry: A new retry instance with the given predicate.
Retry: A new retry instance with the given delay options.
"""
return self._replace(initial=initial, maximum=maximum, multiplier=multiplier)
return type(self)(
predicate=self._predicate,
initial=initial if initial is not None else self._initial,
maximum=maximum if maximum is not None else self._maximum,
multiplier=multiplier if multiplier is not None else self._multiplier,
timeout=self._timeout,
on_error=self._on_error,
)

def __str__(self) -> str:
return (
Expand Down
2 changes: 1 addition & 1 deletion google/api_core/retry/retry_unary.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ class Retry(_BaseRetry):
must be greater than 0.
maximum (float): The maximum amount of time to delay in seconds.
multiplier (float): The multiplier applied to the delay.
timeout (float): How long to keep retrying, in seconds.
timeout (Optional[float]): How long to keep retrying, in seconds.
Note: timeout is only checked before initiating a retry, so the target may
run past the timeout value as long as it is healthy.
on_error (Callable[Exception]): A function to call while processing
Expand Down
47 changes: 34 additions & 13 deletions tests/unit/retry/test_retry_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ def test_constructor_options(self):
assert retry_._on_error is _some_function

@pytest.mark.parametrize("use_deadline", [True, False])
def test_with_timeout(self, use_deadline):
@pytest.mark.parametrize("value", [None, 0, 1, 4, 42, 5.5])
def test_with_timeout(self, use_deadline, value):
retry_ = self._make_one(
predicate=mock.sentinel.predicate,
initial=1,
Expand All @@ -148,11 +149,17 @@ def test_with_timeout(self, use_deadline):
on_error=mock.sentinel.on_error,
)
new_retry = (
retry_.with_timeout(42) if not use_deadline else retry_.with_deadline(42)
retry_.with_timeout(value)
if not use_deadline
else retry_.with_deadline(value)
)
assert retry_ is not new_retry
assert new_retry._timeout == 42
assert new_retry.timeout == 42 if not use_deadline else new_retry.deadline == 42
assert new_retry._timeout == value
assert (
new_retry.timeout == value
if not use_deadline
else new_retry.deadline == value
)

# the rest of the attributes should remain the same
assert new_retry._predicate is retry_._predicate
Expand Down Expand Up @@ -196,20 +203,34 @@ def test_with_delay_noop(self):
assert new_retry._maximum == retry_._maximum
assert new_retry._multiplier == retry_._multiplier

def test_with_delay(self):
@pytest.mark.parametrize(
"originals,updated,expected",
[
[(1, 2, 3), (4, 5, 6), (4, 5, 6)],
[(1, 2, 3), (0, 0, 0), (0, 0, 0)],
[(1, 2, 3), (None, None, None), (1, 2, 3)],
[(0, 0, 0), (None, None, None), (0, 0, 0)],
[(1, 2, 3), (None, 0.5, None), (1, 0.5, 3)],
[(1, 2, 3), (None, 0.5, 4), (1, 0.5, 4)],
[(1, 2, 3), (9, None, None), (9, 2, 3)],
],
)
def test_with_delay(self, originals, updated, expected):
retry_ = self._make_one(
predicate=mock.sentinel.predicate,
initial=1,
maximum=2,
multiplier=3,
timeout=4,
initial=originals[0],
maximum=originals[1],
multiplier=originals[2],
timeout=14,
on_error=mock.sentinel.on_error,
)
new_retry = retry_.with_delay(initial=5, maximum=6, multiplier=7)
new_retry = retry_.with_delay(
initial=updated[0], maximum=updated[1], multiplier=updated[2]
)
assert retry_ is not new_retry
assert new_retry._initial == 5
assert new_retry._maximum == 6
assert new_retry._multiplier == 7
assert new_retry._initial == expected[0]
assert new_retry._maximum == expected[1]
assert new_retry._multiplier == expected[2]

# the rest of the attributes should remain the same
assert new_retry._timeout == retry_._timeout
Expand Down

0 comments on commit 416203c

Please sign in to comment.