[Yarmoluk]

Closes #789.

Motivation

RedisConnectionFactory.sync_to_async_redis passed the sync pool's connection_kwargs straight into AsyncConnectionPool and picked the TLS class with an exact-type check. A client built with Redis(ssl=True, ...), Redis(unix_socket_path=...) or Sentinel.master_for(...) therefore raised TypeError on its first async command (SemanticCache.acheck, and SemanticRouter.aroute once #784 lands), and a subclass of SSLConnection got a plaintext async connection with the password still set.

Changes

• The async connection class is chosen with issubclass: TLS connections (and subclasses) map to AsyncSSLConnection, unix-socket connections to the async UnixDomainSocketConnection, everything else to AsyncConnection.
• _async_connection_kwargs keeps only the keywords the chosen async class accepts, found by walking its __init__ chain (_accepted_init_kwargs), so the filter follows whatever redis-py version is installed rather than a hard-coded key list. A sync-only keyword at its default (None or False) is dropped; one that is set, such as ssl_validate_ocsp=True, raises a ValueError naming it, because the async client cannot honour it and dropping it would silently weaken TLS verification.
• Sentinel-managed pools are rejected up front with a clear ValueError, as RedisCluster already was.
• Seven unit tests in tests/unit/test_sync_to_async_redis.py, none needing a server; five fail on main.

Release Notes

SemanticCache, AsyncSearchIndex and any other async path that converts a caller's sync client now work with clients configured with TLS keywords (Redis(ssl=True, ...)), unix sockets, and custom SSLConnection subclasses. Sentinel-managed clients, RedisCluster clients, and clients with a sync-only setting enabled (OCSP validation) raise ValueError at conversion time with a pointer to redis_url; previously they failed with TypeError on the first command.

Reviewer Brief

Start here: redisvl/redis/connection.py, _async_connection_kwargs. It decides which of the sync pool's settings reach the async connection and which abort the conversion; sync_to_async_redis only picks the class and calls it.

Where it could fail

• redisvl/redis/connection.py:350: a sync-only keyword whose default is not None or False (none exist in redis-py 6.3.0 through 8.1.0) would make every conversion raise; check the value not in (None, False) test against any new keyword a future redis-py adds.
• redisvl/redis/connection.py:328: _accepted_init_kwargs stops at the first __init__ without **kwargs; if redis-py ever adds keywords below AbstractConnection in the chain they would be dropped as unaccepted.

Not verified: cluster mode (unchanged path, raises as before); a live TLS or unix-socket server (the tests stop at make_connection(), where every reported failure occurred).

Beyond CI: the seven new tests pass on redis-py 8.1.0 and on 6.3.0, the floor in pyproject.toml; five of them fail on main. tests/integration/test_llmcache.py, test_embedcache.py and test_async_search_index.py: 140 passed (Redis 8.4 via the repo's testcontainers compose); tests/unit: 1823 passed.

_{Reviewed d23003e · Claude Code (claude-fable-5-1) · lenses: correctness, security, design, tests, docs}

Notes

• A set sync-only keyword raises rather than warns. OCSP validation is a security setting; converting it into a client that cannot validate OCSP, with only a log line (which RedisVL warnings are never shown when the application has not configured logging #786 shows can be invisible), seemed the wrong default.
• Sentinel conversion is rejected rather than rebuilt through AsyncSentinel, as the issue suggests. The pool does carry sentinel_manager and service_name, so a later change could rebuild it; that is a separate feature.
• Once this merges, the SSL/unix-socket caveat in feat(router): add async aroute and aroute_many to SemanticRouter #784's SemanticRouter docstring reduces to Sentinel and cluster; I'll trim it there.

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches connection/TLS conversion used by async cache and index paths; incorrect kwarg handling could weaken TLS or break conversion, though sync-only enabled options now fail loudly instead of silently.

Overview
RedisConnectionFactory.sync_to_async_redis now builds async clients from sync pool settings instead of blindly copying connection_kwargs and using an exact SSLConnection type check.

Connection class selection uses issubclass: TLS (including custom SSLConnection subclasses) → AsyncSSLConnection, unix sockets → AsyncUnixDomainSocketConnection, otherwise AsyncConnection. _async_connection_kwargs filters kwargs by introspecting the async connection class’s __init__ chain; sync-only keys at default (None/False) are dropped, but non-default sync-only settings (e.g. ssl_validate_ocsp=True) raise ValueError so TLS options are not silently weakened. Sentinel-managed pools are rejected up front with a clear error, like RedisCluster.

Seven unit tests cover plain/TLS/URL/unix/Sentinel/OCSP cases without a live Redis server.

^{Reviewed by Cursor Bugbot for commit d23003e. Bugbot is set up for automated code reviews on this repo. Configure here.}

[vishal-bala]

The sync pool fills in some keywords itself, and their defaults aren't None or False. On redis-py 7.4.1, Redis(host="h", protocol=3) raises ValueError: ... does not support maint_notifications_config, maint_notifications_pool_handler, orig_host_address, naming settings the user never touched. On 8.0.1, which the pin allows, every client fails this way, and four of these tests fail with it. Could keywords that redis-py injects itself (maint_notifications_pool_handler, orig_*, and maint_notifications_config unless it's explicitly enabled) be dropped as plumbing, with a protocol=3 case in the tests? While here, value is None or value is False would stop 0 being treated as unset.

[vishal-bala]

Two docstring points:

• "Dropped when they hold their default" isn't quite what the code does; it drops None and False whatever the default is, so "dropped when unset" would be exact (same at line 342).
• It's worth saying that a custom SSLConnection subclass's behaviour isn't carried over; the async side uses the stock TLS class.

Also, #784 has merged, so after a rebase the SemanticRouter.aroute caveat can be trimmed to Sentinel and Cluster here, rather than in a follow-up.

[vishal-bala]

test_tls_url_client_still_converts passes on main and only catches what four other tests do. The rest could fold into two parametrised tests; this version kills the same mutations, plus a few more:

import pytest
from redis import Redis
from redis.asyncio.connection import Connection as AsyncConnection
from redis.asyncio.connection import SSLConnection as AsyncSSLConnection
from redis.asyncio.connection import (
    UnixDomainSocketConnection as AsyncUnixDomainSocketConnection,
)
from redis.connection import ConnectionPool, SSLConnection
from redis.sentinel import Sentinel

from redisvl.redis.connection import RedisConnectionFactory


class _TLS(SSLConnection):
    pass


@pytest.mark.parametrize(
    "client, expected, settings",
    [
        (
            Redis(host="h", port=6380, db=3, decode_responses=True),
            AsyncConnection,
            {"host": "h", "port": 6380, "db": 3, "decode_responses": True},
        ),
        (
            Redis(host="h", ssl=True, password="pw", ssl_check_hostname=True),
            AsyncSSLConnection,
            {"host": "h", "password": "pw", "ssl_check_hostname": True},
        ),
        (
            Redis(connection_pool=ConnectionPool(connection_class=_TLS, host="h")),
            AsyncSSLConnection,
            {"host": "h"},
        ),
        (
            Redis(unix_socket_path="/tmp/redis.sock", db=2),
            AsyncUnixDomainSocketConnection,
            {"path": "/tmp/redis.sock", "db": 2},
        ),
    ],
    ids=["plain", "tls", "tls-subclass", "unix"],
)
def test_converts_to_the_matching_async_connection(client, expected, settings):
    pool = RedisConnectionFactory.sync_to_async_redis(client).connection_pool

    assert type(pool.make_connection()) is expected
    assert settings.items() <= pool.connection_kwargs.items()


@pytest.mark.parametrize(
    "client, match",
    [
        (Sentinel([("localhost", 26379)]).master_for("mymaster"), "Sentinel-managed"),
        (Redis(host="h", ssl=True, ssl_validate_ocsp=True), "ssl_validate_ocsp"),
    ],
    ids=["sentinel", "sync-only-setting"],
)
def test_unconvertible_client_raises(client, match):
    with pytest.raises(ValueError, match=match):
        RedisConnectionFactory.sync_to_async_redis(client)

[vishal-bala]

A sync Retry passes the filter unchanged, and the async connection awaits a coroutine it returns, so the retry policy silently never runs. I measured a converted client failing after CLIENT KILL where a native async client recovers. It's no worse than main, so it's fine as a follow-up if you'd rather not take it on here.