Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 15 additions & 3 deletions docs/agents.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ Everything in this table is importable from `redis_client_kit` itself.
| `check_redis_health` | `(client)` | `bool` — never raises |
| `close_async_redis_client` | `await (client)` | `None` — shielded, 10 s timeout, never raises |
| `close_redis_client` | `(client)` | `None` — never raises |
| `build_base_redis_kwargs` | `(settings)` | `dict[str, object]` of `redis-py` keyword arguments |
| `build_redis_retry` | `(settings)` | `redis.retry.Retry` — `Retry(NoBackoff(), 0)` when retries are off |
| `build_base_redis_kwargs` | `(settings, asyncio=False)` | `dict[str, object]` of `redis-py` keyword arguments; `asyncio=True` for a `redis.asyncio` client |
| `build_redis_retry` | `(settings, asyncio=False)` | `redis.retry.Retry`, or `redis.asyncio.retry.Retry` with `asyncio=True` — `Retry(NoBackoff(), 0)` when retries are off |
| `parse_redis_url_node` | `(node)` | `tuple[str, int]` — `ValueError` on a node with no port |
| `AsyncRedisClient` | type alias | `redis.asyncio.Redis | redis.asyncio.cluster.RedisCluster` |
| `SyncRedisClient` | type alias | `redis.Redis | redis.cluster.RedisCluster` |
Expand Down Expand Up @@ -309,7 +309,11 @@ See rules 15 to 17.
`Retry(NoBackoff(), 0)` explicitly — never nothing, because `redis-py` given no
`Retry` retries on its own since 6.0 (three times; ten since 8.0, with jittered
backoff), which turns a `socket_timeout=0.5` failure into ten seconds or more. With
retries off, the first `ConnectionError` or `TimeoutError` is the one you get.
retries off, the first `ConnectionError` or `TimeoutError` is the one you get. The
flavour follows the client: the async factory hands `redis.asyncio.retry.Retry`, the
sync factory `redis.retry.Retry`, and both helpers build the sync class unless called
with `asyncio=True`. A sync `Retry` on a `redis.asyncio` client never retries — its
`call_with_retry` does not await, so the failure never reaches the loop.
6. **Retries cover connection failures, not command failures.** `redis-py`'s `Retry`
defaults to `ConnectionError`, `TimeoutError` and `socket.timeout`; a `ResponseError`
from a bad command is raised on the first try. The delay is
Expand Down Expand Up @@ -404,6 +408,14 @@ BaseRedisSettings(
)
```

```python
# WRONG — a sync Retry on an async client never retries: its call_with_retry does not await
client = redis.asyncio.Redis(**build_base_redis_kwargs(settings), host="localhost", port=6379)

# RIGHT — asyncio=True builds redis.asyncio.retry.Retry, which is what the async factory does
client = redis.asyncio.Redis(**build_base_redis_kwargs(settings, asyncio=True), host="localhost", port=6379)
```

```python
# WRONG — metrics_enabled does nothing, and the client is never instrumented
settings = BaseRedisSettings(key_prefix="myapp", metrics_enabled=True)
Expand Down
6 changes: 6 additions & 0 deletions docs/guide/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ or more. Disabled means the first `ConnectionError` or `TimeoutError` is raised
with `socket_timeout=0.5`, a command against an unreachable Redis fails in about half a
second.

The async factory hands redis-py `redis.asyncio.retry.Retry` and the sync factory
`redis.retry.Retry`; they are not interchangeable, because the sync class's
`call_with_retry` does not await, so on a `redis.asyncio` client it never retries. If you
assemble the keyword arguments yourself, `build_base_redis_kwargs(settings, asyncio=True)`
and `build_redis_retry(settings, asyncio=True)` build the async flavour.

Retry logic uses exponential backoff:
```
delay = min(backoff_cap, backoff_base * (2 ** failures))
Expand Down
4 changes: 2 additions & 2 deletions redis_client_kit/aio/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def _create_async_single_client(
extra={"host": settings.connection.host, "port": settings.connection.port},
)

client_kwargs: dict[str, Any] = build_base_redis_kwargs(settings)
client_kwargs: dict[str, Any] = build_base_redis_kwargs(settings, asyncio=True)
client_kwargs.update(
{
"host": settings.connection.host,
Expand Down Expand Up @@ -87,7 +87,7 @@ def _create_async_cluster_client(
},
)

cluster_kwargs: dict[str, Any] = build_base_redis_kwargs(settings)
cluster_kwargs: dict[str, Any] = build_base_redis_kwargs(settings, asyncio=True)
cluster_kwargs.update(
{
"startup_nodes": startup_nodes,
Expand Down
35 changes: 29 additions & 6 deletions redis_client_kit/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import base64
import binascii
from pathlib import Path
from typing import Literal, overload
from urllib.parse import urlparse

from redis.asyncio.retry import Retry as AsyncRetry
from redis.backoff import ExponentialBackoff, NoBackoff
from redis.retry import Retry

Expand Down Expand Up @@ -34,8 +36,12 @@ def parse_redis_url_node(node: str) -> tuple[str, int]:
return host, port


def build_base_redis_kwargs(settings: RedisSettingsProtocol) -> dict[str, object]:
"""Build base Redis client keyword arguments."""
def build_base_redis_kwargs(settings: RedisSettingsProtocol, *, asyncio: bool = False) -> dict[str, object]:
"""Build base Redis client keyword arguments.

``asyncio=True`` builds them for a ``redis.asyncio`` client, whose ``retry`` has to be
``redis.asyncio.retry.Retry``.
"""
if settings.ssl.enabled:
if settings.ssl.ca_certs:
_validate_pem_format(settings.ssl.ca_certs, "CERTIFICATE")
Expand All @@ -52,7 +58,7 @@ def build_base_redis_kwargs(settings: RedisSettingsProtocol) -> dict[str, object
"socket_keepalive": settings.pool.socket_keepalive,
"socket_keepalive_options": settings.pool.socket_keepalive_options,
"health_check_interval": settings.health_check_interval,
"retry": build_redis_retry(settings),
"retry": build_redis_retry(settings, asyncio=asyncio),
"decode_responses": settings.response.decode_responses,
"encoding": settings.response.encoding,
"client_name": settings.connection.client_name,
Expand All @@ -72,22 +78,39 @@ def build_base_redis_kwargs(settings: RedisSettingsProtocol) -> dict[str, object
return kwargs


def build_redis_retry(settings: RedisSettingsProtocol) -> Retry:
@overload
def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: Literal[False] = ...) -> Retry: ...


@overload
def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: Literal[True]) -> AsyncRetry: ...


@overload
def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: bool) -> Retry | AsyncRetry: ...


def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: bool = False) -> Retry | AsyncRetry:
"""Build Redis Retry object from settings.

Retries are off unless both ``retry.enabled`` and ``retry.max_attempts`` are set. Off is
still a ``Retry``, with zero retries: handed nothing, redis-py retries on its own (three
times since 6.0, ten since 8.0, with jittered backoff).

``asyncio=True`` builds ``redis.asyncio.retry.Retry``, the one a ``redis.asyncio`` client
needs: the sync class's ``call_with_retry`` does not await, so on an async client it
never retries.
"""
retry_class = AsyncRetry if asyncio else Retry
if settings.retry.enabled and settings.retry.max_attempts:
return Retry(
return retry_class(
backoff=ExponentialBackoff(
cap=settings.retry.backoff_cap,
base=settings.retry.backoff_base,
),
retries=settings.retry.max_attempts,
)
return Retry(backoff=NoBackoff(), retries=0)
return retry_class(backoff=NoBackoff(), retries=0)


def _validate_pem_format(path: str | Path, file_type: str) -> None:
Expand Down
65 changes: 65 additions & 0 deletions tests/integration/test_redis_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,39 @@ async def test__async_redis_client__retry_disabled_and_redis_paused__raises_with
assert elapsed < 2.0


@pytest.mark.asyncio
async def test__async_redis_client__retry_enabled_and_redis_paused__retries_before_raising(
redis_container: RedisContainer,
) -> None:
# Arrange
settings = FakeRedisSettings()
settings.connection.host = redis_container.get_container_host_ip()
settings.connection.port = int(redis_container.get_exposed_port(redis_container.port))
settings.pool.socket_timeout = 0.5
settings.pool.socket_connect_timeout = 0.5
settings.retry.enabled = True
settings.retry.max_attempts = 3
settings.retry.backoff_base = 0.1
settings.retry.backoff_cap = 1.0
client = create_async_redis_client(settings)
container = redis_container.get_wrapped_container()
await client.ping()

# Act
container.pause()
try:
started = time.perf_counter()
with pytest.raises(RedisTimeoutError):
await client.get("paused")
elapsed = time.perf_counter() - started
finally:
container.unpause()
await close_async_redis_client(client)

# Assert - four attempts of 0.5 s plus 0.2 + 0.4 + 0.8 s of backoff, not a single attempt
assert 3.0 <= elapsed < 6.0


def test__sync_redis_client__retry_disabled_and_redis_paused__raises_within_socket_timeout(
redis_container: RedisContainer,
) -> None:
Expand Down Expand Up @@ -134,3 +167,35 @@ def test__sync_redis_client__retry_disabled_and_redis_paused__raises_within_sock

# Assert
assert elapsed < 2.0


def test__sync_redis_client__retry_enabled_and_redis_paused__retries_before_raising(
redis_container: RedisContainer,
) -> None:
# Arrange
settings = FakeRedisSettings()
settings.connection.host = redis_container.get_container_host_ip()
settings.connection.port = int(redis_container.get_exposed_port(redis_container.port))
settings.pool.socket_timeout = 0.5
settings.pool.socket_connect_timeout = 0.5
settings.retry.enabled = True
settings.retry.max_attempts = 3
settings.retry.backoff_base = 0.1
settings.retry.backoff_cap = 1.0
client = create_redis_client(settings)
container = redis_container.get_wrapped_container()
client.ping()

# Act
container.pause()
try:
started = time.perf_counter()
with pytest.raises(RedisTimeoutError):
client.get("paused")
elapsed = time.perf_counter() - started
finally:
container.unpause()
close_redis_client(client)

# Assert - four attempts of 0.5 s plus 0.2 + 0.4 + 0.8 s of backoff, not a single attempt
assert 3.0 <= elapsed < 6.0
2 changes: 1 addition & 1 deletion tests/unit/aio/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

import pytest
from redis.asyncio.cluster import RedisCluster
from redis.asyncio.retry import Retry
from redis.backoff import NoBackoff
from redis.exceptions import RedisError
from redis.retry import Retry

from redis_client_kit.aio import (
check_async_redis_health,
Expand Down
24 changes: 17 additions & 7 deletions tests/unit/aio/test_resilience.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,37 @@

import pytest
from redis.asyncio import RedisCluster
from redis.asyncio.retry import Retry
from redis.exceptions import BusyLoadingError, ClusterDownError, TimeoutError
from redis.retry import Retry

from redis_client_kit import check_async_redis_health, create_async_redis_client


def test__create_async_redis_client__retry_enabled__passes_retry_to_client(mock_redis_settings: MagicMock) -> None:
@pytest.mark.parametrize(
"cluster_mode, expected_class",
[
(False, "InstrumentedRedis"),
(True, "InstrumentedRedisCluster"),
],
ids=["single-node", "cluster"],
)
def test__create_async_redis_client__retry_enabled__passes_async_retry_to_client(
mock_redis_settings: MagicMock, cluster_mode: bool, expected_class: str
) -> None:
# Arrange
mock_redis_settings.cluster.enabled = cluster_mode
mock_redis_settings.retry.enabled = True
mock_redis_settings.retry.max_attempts = 5
mock_redis_settings.retry_backoff_base = 0.5
mock_metrics = MagicMock()

# Act
with patch("redis_client_kit.aio.factory.InstrumentedRedis") as mock_redis:
with patch(f"redis_client_kit.aio.factory.{expected_class}") as mock_class:
create_async_redis_client(mock_redis_settings, metrics=mock_metrics)

# Assert
kwargs = mock_redis.call_args[1]
assert "retry" in kwargs
assert isinstance(kwargs["retry"], Retry)
retry = mock_class.call_args[1]["retry"]
assert isinstance(retry, Retry)
assert retry._retries == 5


def test__create_async_redis_client__ssl_enabled__passes_ssl_config_to_client(mock_redis_settings: MagicMock) -> None:
Expand Down
39 changes: 37 additions & 2 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest.mock import MagicMock, patch

import pytest
from redis.asyncio.retry import Retry as AsyncRetry
from redis.backoff import NoBackoff
from redis.retry import Retry

Expand Down Expand Up @@ -68,8 +69,34 @@ def test__build_redis_retry__retry_enabled__returns_retry_object(mock_redis_sett
retry = build_redis_retry(mock_redis_settings)

# Assert
assert retry is not None
assert getattr(retry, "_retries", 0) == 5
assert isinstance(retry, Retry)
assert retry._retries == 5


def test__build_redis_retry__asyncio_retry_enabled__returns_async_retry_object(mock_redis_settings: MagicMock) -> None:
# Arrange
mock_redis_settings.retry.enabled = True
mock_redis_settings.retry.max_attempts = 5

# Act
retry = build_redis_retry(mock_redis_settings, asyncio=True)

# Assert
assert isinstance(retry, AsyncRetry)
assert retry._retries == 5


def test__build_redis_retry__asyncio_retry_disabled__returns_async_zero_retries(mock_redis_settings: MagicMock) -> None:
# Arrange
mock_redis_settings.retry.enabled = False

# Act
retry = build_redis_retry(mock_redis_settings, asyncio=True)

# Assert
assert isinstance(retry, AsyncRetry)
assert retry._retries == 0
assert isinstance(retry._backoff, NoBackoff)


def test__build_redis_retry__no_max_attempts__returns_zero_retries(mock_redis_settings: MagicMock) -> None:
Expand Down Expand Up @@ -121,6 +148,14 @@ def test__build_base_redis_kwargs__retry_disabled__passes_zero_retries(
assert isinstance(retry._backoff, NoBackoff)


def test__build_base_redis_kwargs__asyncio__passes_async_retry(mock_redis_settings: MagicMock) -> None:
# Act
kwargs = build_base_redis_kwargs(mock_redis_settings, asyncio=True)

# Assert
assert isinstance(kwargs["retry"], AsyncRetry)


def test__build_base_redis_kwargs__ssl_enabled__includes_ssl_parameters(mock_redis_settings: MagicMock) -> None:
# Arrange
mock_redis_settings.ssl.enabled = True
Expand Down