Skip to content

Commit

Permalink
Rename RetryStrategy to Strategy
Browse files Browse the repository at this point in the history
The `Retry` part is in the module name, so it is redundant. This also
change imports to use the module name, and change some variables to
`strategy` to make the code more clear (before the style was not very
consistent, sometimes calling variables `retry_spec` or just `retry`.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
  • Loading branch information
llucax committed Feb 29, 2024
1 parent 9d35469 commit 9631e92
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 52 deletions.
6 changes: 3 additions & 3 deletions src/frequenz/client/base/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"""Default retry jitter, in seconds."""


class RetryStrategy(ABC):
class Strategy(ABC):
"""Interface for implementing retry strategies."""

_limit: int | None
Expand Down Expand Up @@ -74,7 +74,7 @@ def __iter__(self) -> Iterator[float]:
yield interval


class LinearBackoff(RetryStrategy):
class LinearBackoff(Strategy):
"""Provides methods for calculating the interval between retries."""

def __init__(
Expand Down Expand Up @@ -112,7 +112,7 @@ def next_interval(self) -> float | None:
return self._interval + random.uniform(0.0, self._jitter)


class ExponentialBackoff(RetryStrategy):
class ExponentialBackoff(Strategy):
"""Provides methods for calculating the exponential interval between retries."""

DEFAULT_INTERVAL = DEFAULT_RETRY_INTERVAL
Expand Down
14 changes: 7 additions & 7 deletions src/frequenz/client/base/streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def __init__(
stream_name: str,
stream_method: Callable[[], grpc.aio.UnaryStreamCall[Any, InputT]],
transform: Callable[[InputT], OutputT],
retry_spec: retry.RetryStrategy | None = None,
retry_strategy: retry.Strategy | None = None,
):
"""Initialize the streaming helper.
Expand All @@ -40,14 +40,14 @@ def __init__(
stream_method: A function that returns the grpc stream. This function is
called everytime the connection is lost and we want to retry.
transform: A function to transform the input type to the output type.
retry_spec: The retry strategy to use, when the connection is lost. Defaults
retry_strategy: The retry strategy to use, when the connection is lost. Defaults
to retries every 3 seconds, with a jitter of 1 second, indefinitely.
"""
self._stream_name = stream_name
self._stream_method = stream_method
self._transform = transform
self._retry_spec = (
retry.LinearBackoff() if retry_spec is None else retry_spec.copy()
self._retry_strategy = (
retry.LinearBackoff() if retry_strategy is None else retry_strategy.copy()
)

self._channel: channels.Broadcast[OutputT] = channels.Broadcast(
Expand Down Expand Up @@ -92,19 +92,19 @@ async def _run(self) -> None:
_logger.exception(
"Error in grpc streaming method: %s", self._stream_name
)
if interval := self._retry_spec.next_interval():
if interval := self._retry_strategy.next_interval():
_logger.warning(
"`%s`, connection ended, retrying %s in %0.3f seconds.",
self._stream_name,
self._retry_spec.get_progress(),
self._retry_strategy.get_progress(),
interval,
)
await asyncio.sleep(interval)
else:
_logger.warning(
"`%s`, connection ended, retry limit exceeded %s.",
self._stream_name,
self._retry_spec.get_progress(),
self._retry_strategy.get_progress(),
)
await self._channel.close()
break
84 changes: 42 additions & 42 deletions tests/retry_strategy/test_retry_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

# pylint: disable=chained-comparison

from frequenz.client.base.retry import ExponentialBackoff, LinearBackoff, RetryStrategy
from frequenz.client.base import retry


class TestLinearBackoff:
Expand All @@ -16,48 +16,48 @@ def test_no_limit(self) -> None:
interval = 3
jitter = 0
limit = None
retry = LinearBackoff(interval=interval, jitter=jitter, limit=limit)
strategy = retry.LinearBackoff(interval=interval, jitter=jitter, limit=limit)

for _ in range(10):
assert retry.next_interval() == interval
assert strategy.next_interval() == interval

def test_iter(self) -> None:
"""Test iterator."""
assert list(LinearBackoff(1, 0, 3)) == [1, 1, 1]
assert list(retry.LinearBackoff(1, 0, 3)) == [1, 1, 1]

def test_with_limit(self) -> None:
"""Test limit works."""
interval = 3
jitter = 0
limit = 5
retry: RetryStrategy = LinearBackoff(
strategy: retry.Strategy = retry.LinearBackoff(
interval=interval, jitter=jitter, limit=limit
)

for _ in range(limit):
assert retry.next_interval() == interval
assert retry.next_interval() is None
assert strategy.next_interval() == interval
assert strategy.next_interval() is None

retry.reset()
strategy.reset()
for _ in range(limit - 1):
assert retry.next_interval() == interval
retry.reset()
assert strategy.next_interval() == interval
strategy.reset()
for _ in range(limit):
assert retry.next_interval() == interval
assert retry.next_interval() is None
assert strategy.next_interval() == interval
assert strategy.next_interval() is None

def test_with_jitter_no_limit(self) -> None:
"""Test with jitter but no limit."""
interval = 3
jitter = 1
limit = None
retry: RetryStrategy = LinearBackoff(
strategy: retry.Strategy = retry.LinearBackoff(
interval=interval, jitter=jitter, limit=limit
)

prev = 0.0
for _ in range(5):
next_val = retry.next_interval()
next_val = strategy.next_interval()
assert next_val is not None
assert next_val > interval and next_val < (interval + jitter)
assert next_val != prev
Expand All @@ -68,30 +68,30 @@ def test_with_jitter_with_limit(self) -> None:
interval = 3
jitter = 1
limit = 2
retry: RetryStrategy = LinearBackoff(
strategy: retry.Strategy = retry.LinearBackoff(
interval=interval, jitter=jitter, limit=limit
)

prev = 0.0
for _ in range(2):
next_val = retry.next_interval()
next_val = strategy.next_interval()
assert next_val is not None
assert next_val > interval and next_val < (interval + jitter)
assert next_val != prev
prev = next_val
assert retry.next_interval() is None
assert strategy.next_interval() is None

retry.reset()
next_val = retry.next_interval()
strategy.reset()
next_val = strategy.next_interval()
assert next_val is not None
assert next_val > interval and next_val < (interval + jitter)
assert next_val != prev

def test_deep_copy(self) -> None:
"""Test if deep copies are really deep copies."""
retry = LinearBackoff(1.0, 0.0, 2)
strategy = retry.LinearBackoff(1.0, 0.0, 2)

copy1 = retry.copy()
copy1 = strategy.copy()
assert copy1.next_interval() == 1.0
assert copy1.next_interval() == 1.0
assert copy1.next_interval() is None
Expand All @@ -108,29 +108,29 @@ class TestExponentialBackoff:

def test_no_limit(self) -> None:
"""Test base case."""
retry = ExponentialBackoff(3, 30, 2, 0.0)
strategy = retry.ExponentialBackoff(3, 30, 2, 0.0)

assert retry.next_interval() == 3.0
assert retry.next_interval() == 6.0
assert retry.next_interval() == 12.0
assert retry.next_interval() == 24.0
assert retry.next_interval() == 30.0
assert retry.next_interval() == 30.0
assert strategy.next_interval() == 3.0
assert strategy.next_interval() == 6.0
assert strategy.next_interval() == 12.0
assert strategy.next_interval() == 24.0
assert strategy.next_interval() == 30.0
assert strategy.next_interval() == 30.0

def test_with_limit(self) -> None:
"""Test limit works."""
retry = ExponentialBackoff(3, jitter=0.0, limit=3)
strategy = retry.ExponentialBackoff(3, jitter=0.0, limit=3)

assert retry.next_interval() == 3.0
assert retry.next_interval() == 6.0
assert retry.next_interval() == 12.0
assert retry.next_interval() is None
assert strategy.next_interval() == 3.0
assert strategy.next_interval() == 6.0
assert strategy.next_interval() == 12.0
assert strategy.next_interval() is None

def test_deep_copy(self) -> None:
"""Test if deep copies are really deep copies."""
retry = ExponentialBackoff(3.0, 30.0, 2, 0.0, 2)
strategy = retry.ExponentialBackoff(3.0, 30.0, 2, 0.0, 2)

copy1 = retry.copy()
copy1 = strategy.copy()
assert copy1.next_interval() == 3.0
assert copy1.next_interval() == 6.0
assert copy1.next_interval() is None
Expand All @@ -148,7 +148,7 @@ def test_with_jitter_no_limit(self) -> None:
jitter = 1
multiplier = 2
limit = None
retry: RetryStrategy = ExponentialBackoff(
strategy: retry.Strategy = retry.ExponentialBackoff(
initial_interval=initial_interval,
max_interval=max_interval,
multiplier=multiplier,
Expand All @@ -158,7 +158,7 @@ def test_with_jitter_no_limit(self) -> None:

prev = 0.0
for count in range(5):
next_val = retry.next_interval()
next_val = strategy.next_interval()
exp_backoff_interval = initial_interval * multiplier**count
assert next_val is not None
assert initial_interval <= next_val <= max_interval
Expand All @@ -174,7 +174,7 @@ def test_with_jitter_with_limit(self) -> None:
jitter = 1
multiplier = 2
limit = 2
retry: RetryStrategy = ExponentialBackoff(
strategy: retry.Strategy = retry.ExponentialBackoff(
initial_interval=initial_interval,
max_interval=max_interval,
multiplier=multiplier,
Expand All @@ -184,18 +184,18 @@ def test_with_jitter_with_limit(self) -> None:

prev = 0.0
for count in range(2):
next_val = retry.next_interval()
next_val = strategy.next_interval()
exp_backoff_interval = initial_interval * multiplier**count
assert next_val is not None
assert initial_interval <= next_val <= max_interval
assert next_val >= min(exp_backoff_interval, max_interval)
assert next_val <= min(exp_backoff_interval + jitter, max_interval)
assert next_val != prev
prev = next_val
assert retry.next_interval() is None
assert strategy.next_interval() is None

retry.reset()
next_val = retry.next_interval()
strategy.reset()
next_val = strategy.next_interval()
count = 0
exp_backoff_interval = initial_interval * multiplier**count
assert next_val is not None
Expand Down

0 comments on commit 9631e92

Please sign in to comment.