Skip to content
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.

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

[4.6.1] Supporting setting CLIENT INFO via CLIENT SETINFO #2875

Merged
merged 1 commit into from
Aug 8, 2023
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
20 changes: 19 additions & 1 deletion redis/asyncio/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
TimeoutError,
)
from redis.typing import EncodableT, EncodedT
from redis.utils import HIREDIS_AVAILABLE, str_if_bytes
from redis.utils import HIREDIS_AVAILABLE, get_lib_version, str_if_bytes

hiredis = None
if HIREDIS_AVAILABLE:
Expand Down Expand Up @@ -453,6 +453,8 @@ class AbstractConnection:
"db",
"username",
"client_name",
"lib_name",
"lib_version",
"credential_provider",
"password",
"socket_timeout",
Expand Down Expand Up @@ -491,6 +493,8 @@ def __init__(
socket_read_size: int = 65536,
health_check_interval: float = 0,
client_name: Optional[str] = None,
lib_name: Optional[str] = "redis-py",
lib_version: Optional[str] = get_lib_version(),
username: Optional[str] = None,
retry: Optional[Retry] = None,
redis_connect_func: Optional[ConnectCallbackT] = None,
Expand All @@ -507,6 +511,8 @@ def __init__(
self.pid = os.getpid()
self.db = db
self.client_name = client_name
self.lib_name = lib_name
self.lib_version = lib_version
self.credential_provider = credential_provider
self.password = password
self.username = username
Expand Down Expand Up @@ -654,6 +660,18 @@ async def on_connect(self) -> None:
if str_if_bytes(await self.read_response()) != "OK":
raise ConnectionError("Error setting client name")

try:
# set the library name and version
if self.lib_name:
await self.send_command("CLIENT", "SETINFO", "LIB-NAME", self.lib_name)
await self.read_response()
if self.lib_version:
await self.send_command(
"CLIENT", "SETINFO", "LIB-VER", self.lib_version
)
await self.read_response()
except ResponseError:
pass
# if a database is specified, switch to it
if self.db:
await self.send_command("SELECT", self.db)
Expand Down
11 changes: 10 additions & 1 deletion redis/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
)
from redis.lock import Lock
from redis.retry import Retry
from redis.utils import safe_str, str_if_bytes
from redis.utils import get_lib_version, safe_str, str_if_bytes

SYM_EMPTY = b""
EMPTY_RESPONSE = "EMPTY_RESPONSE"
Expand Down Expand Up @@ -643,7 +643,11 @@ def parse_client_info(value):
"key1=value1 key2=value2 key3=value3"
"""
client_info = {}
value = str_if_bytes(value)
if value[-1] == "\n":
value = value[:-1]
infos = str_if_bytes(value).split(" ")
infos = value.split(" ")
for info in infos:
key, value = info.split("=")
client_info[key] = value
Expand Down Expand Up @@ -754,6 +758,7 @@ class AbstractRedis:
"CLIENT SETNAME": bool_ok,
"CLIENT UNBLOCK": lambda r: r and int(r) == 1 or False,
"CLIENT PAUSE": bool_ok,
"CLIENT SETINFO": bool_ok,
"CLIENT GETREDIR": int,
"CLIENT TRACKINGINFO": lambda r: list(map(str_if_bytes, r)),
"CLUSTER ADDSLOTS": bool_ok,
Expand Down Expand Up @@ -949,6 +954,8 @@ def __init__(
single_connection_client=False,
health_check_interval=0,
client_name=None,
lib_name="redis-py",
lib_version=get_lib_version(),
username=None,
retry=None,
redis_connect_func=None,
Expand Down Expand Up @@ -999,6 +1006,8 @@ def __init__(
"max_connections": max_connections,
"health_check_interval": health_check_interval,
"client_name": client_name,
"lib_name": lib_name,
"lib_version": lib_version,
"redis_connect_func": redis_connect_func,
"credential_provider": credential_provider,
}
Expand Down
3 changes: 3 additions & 0 deletions redis/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ def parse_cluster_myshardid(resp, **options):
"encoding_errors",
"errors",
"host",
"lib_name",
"lib_version",
"max_connections",
"nodes_flag",
"redis_connect_func",
Expand Down Expand Up @@ -220,6 +222,7 @@ class AbstractRedisCluster:
"CLIENT LIST",
"CLIENT SETNAME",
"CLIENT GETNAME",
"CLIENT SETINFO",
"CONFIG SET",
"CONFIG REWRITE",
"CONFIG RESETSTAT",
Expand Down
7 changes: 7 additions & 0 deletions redis/commands/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,13 @@ def client_setname(self, name: str, **kwargs) -> ResponseT:
"""
return self.execute_command("CLIENT SETNAME", name, **kwargs)

def client_setinfo(self, attr: str, value: str, **kwargs) -> ResponseT:
"""
Sets the current connection library name or version
For mor information see https://redis.io/commands/client-setinfo
"""
return self.execute_command("CLIENT SETINFO", attr, value, **kwargs)

def client_unblock(
self, client_id: int, error: bool = False, **kwargs
) -> ResponseT:
Expand Down
5 changes: 5 additions & 0 deletions redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
CRYPTOGRAPHY_AVAILABLE,
HIREDIS_AVAILABLE,
HIREDIS_PACK_AVAILABLE,
get_lib_version,
str_if_bytes,
)

Expand Down Expand Up @@ -605,6 +606,8 @@ def __init__(
socket_read_size=65536,
health_check_interval=0,
client_name=None,
lib_name="redis-py",
lib_version=get_lib_version(),
username=None,
retry=None,
redis_connect_func=None,
Expand All @@ -628,6 +631,8 @@ def __init__(
self.pid = os.getpid()
self.db = db
self.client_name = client_name
self.lib_name = lib_name
self.lib_version = lib_version
self.credential_provider = credential_provider
self.password = password
self.username = username
Expand Down
13 changes: 13 additions & 0 deletions redis/utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import sys
from contextlib import contextmanager
from functools import wraps
from typing import Any, Dict, Mapping, Union
Expand All @@ -12,6 +13,10 @@
HIREDIS_AVAILABLE = False
HIREDIS_PACK_AVAILABLE = False

if sys.version_info >= (3, 8):
from importlib import metadata
else:
import importlib_metadata as metadata
try:
import cryptography # noqa

Expand Down Expand Up @@ -110,3 +115,11 @@ def wrapper(*args, **kwargs):
return wrapper

return decorator


def get_lib_version():
try:
libver = metadata.version("redis")
except metadata.PackageNotFoundError:
libver = "99.99.99"
return libver
5 changes: 2 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ def pytest_sessionstart(session):
enterprise = info["enterprise"]
except redis.ConnectionError:
# provide optimistic defaults
info = {}
version = "10.0.0"
arch_bits = 64
cluster_enabled = False
Expand All @@ -157,9 +158,7 @@ def pytest_sessionstart(session):
redismod_url = session.config.getoption("--redismod-url")
info = _get_info(redismod_url)
REDIS_INFO["modules"] = info["modules"]
except redis.exceptions.ConnectionError:
pass
except KeyError:
except (KeyError, redis.exceptions.ConnectionError):
pass

if cluster_enabled:
Expand Down
12 changes: 12 additions & 0 deletions tests/test_asyncio/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,15 @@ async def __aexit__(self, exc_type, exc_inst, tb):

def asynccontextmanager(func):
return _asynccontextmanager(func)


# helpers to get the connection arguments for this run
@pytest.fixture()
def redis_url(request):
return request.config.getoption("--redis-url")


@pytest.fixture()
def connect_args(request):
url = request.config.getoption("--redis-url")
return parse_url(url)
2 changes: 1 addition & 1 deletion tests/test_asyncio/test_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -2186,7 +2186,7 @@ async def test_acl_log(
await user_client.hset("{cache}:0", "hkey", "hval")

assert isinstance(await r.acl_log(target_nodes=node), list)
assert len(await r.acl_log(target_nodes=node)) == 2
assert len(await r.acl_log(target_nodes=node)) == 3
assert len(await r.acl_log(count=1, target_nodes=node)) == 1
assert isinstance((await r.acl_log(target_nodes=node))[0], dict)
assert "client-info" in (await r.acl_log(count=1, target_nodes=node))[0]
Expand Down
25 changes: 23 additions & 2 deletions tests/test_asyncio/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async def test_acl_deluser(self, r_teardown):
username = "redis-py-user"
r = r_teardown(username)

assert await r.acl_deluser(username) == 0
assert await r.acl_deluser(username) in [0, 1]
assert await r.acl_setuser(username, enabled=False, reset=True)
assert await r.acl_deluser(username) == 1

Expand Down Expand Up @@ -268,7 +268,7 @@ async def test_acl_log(self, r_teardown, create_redis):
await user_client.hset("cache:0", "hkey", "hval")

assert isinstance(await r.acl_log(), list)
assert len(await r.acl_log()) == 2
assert len(await r.acl_log()) == 3
assert len(await r.acl_log(count=1)) == 1
assert isinstance((await r.acl_log())[0], dict)
assert "client-info" in (await r.acl_log(count=1))[0]
Expand Down Expand Up @@ -347,6 +347,27 @@ async def test_client_setname(self, r: redis.Redis):
assert await r.client_setname("redis_py_test")
assert await r.client_getname() == "redis_py_test"

@skip_if_server_version_lt("7.2.0")
async def test_client_setinfo(self, r: redis.Redis):
await r.ping()
info = await r.client_info()
assert info["lib-name"] == "redis-py"
assert info["lib-ver"] == redis.__version__
assert await r.client_setinfo("lib-name", "test")
assert await r.client_setinfo("lib-ver", "123")

info = await r.client_info()
assert info["lib-name"] == "test"
assert info["lib-ver"] == "123"
r2 = redis.asyncio.Redis(lib_name="test2", lib_version="1234")
info = await r2.client_info()
assert info["lib-name"] == "test2"
assert info["lib-ver"] == "1234"
r3 = redis.asyncio.Redis(lib_name=None, lib_version=None)
info = await r3.client_info()
assert info["lib-name"] == ""
assert info["lib-ver"] == ""

@skip_if_server_version_lt("2.6.9")
@pytest.mark.onlynoncluster
async def test_client_kill(self, r: redis.Redis, r2):
Expand Down
86 changes: 4 additions & 82 deletions tests/test_asyncio/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
from redis.asyncio.connection import (
BaseParser,
Connection,
HiredisParser,
PythonParser,
UnixDomainSocketConnection,
)
from redis.asyncio.retry import Retry
from redis.backoff import NoBackoff
from redis.exceptions import ConnectionError, InvalidResponse, TimeoutError
from redis.utils import HIREDIS_AVAILABLE
from tests.conftest import skip_if_server_version_lt

from .compat import mock
Expand Down Expand Up @@ -126,9 +124,11 @@ async def test_can_run_concurrent_commands(r):
assert all(await asyncio.gather(*(r.ping() for _ in range(10))))


async def test_connect_retry_on_timeout_error():
async def test_connect_retry_on_timeout_error(connect_args):
"""Test that the _connect function is retried in case of a timeout"""
conn = Connection(retry_on_timeout=True, retry=Retry(NoBackoff(), 3))
conn = Connection(
retry_on_timeout=True, retry=Retry(NoBackoff(), 3), **connect_args
)
origin_connect = conn._connect
conn._connect = mock.AsyncMock()

Expand Down Expand Up @@ -195,84 +195,6 @@ async def test_connection_parse_response_resume(r: redis.Redis):
assert i > 0


@pytest.mark.onlynoncluster
@pytest.mark.parametrize(
"parser_class", [PythonParser, HiredisParser], ids=["PythonParser", "HiredisParser"]
)
async def test_connection_disconect_race(parser_class):
"""
This test reproduces the case in issue #2349
where a connection is closed while the parser is reading to feed the
internal buffer.The stream `read()` will succeed, but when it returns,
another task has already called `disconnect()` and is waiting for
close to finish. When we attempts to feed the buffer, we will fail
since the buffer is no longer there.

This test verifies that a read in progress can finish even
if the `disconnect()` method is called.
"""
if parser_class == HiredisParser and not HIREDIS_AVAILABLE:
pytest.skip("Hiredis not available")

args = {}
args["parser_class"] = parser_class

conn = Connection(**args)

cond = asyncio.Condition()
# 0 == initial
# 1 == reader is reading
# 2 == closer has closed and is waiting for close to finish
state = 0

# Mock read function, which wait for a close to happen before returning
# Can either be invoked as two `read()` calls (HiredisParser)
# or as a `readline()` followed by `readexact()` (PythonParser)
chunks = [b"$13\r\n", b"Hello, World!\r\n"]

async def read(_=None):
nonlocal state
async with cond:
if state == 0:
state = 1 # we are reading
cond.notify()
# wait until the closing task has done
await cond.wait_for(lambda: state == 2)
return chunks.pop(0)

# function closes the connection while reader is still blocked reading
async def do_close():
nonlocal state
async with cond:
await cond.wait_for(lambda: state == 1)
state = 2
cond.notify()
await conn.disconnect()

async def do_read():
return await conn.read_response()

reader = mock.AsyncMock()
writer = mock.AsyncMock()
writer.transport = mock.Mock()
writer.transport.get_extra_info.side_effect = None

# for HiredisParser
reader.read.side_effect = read
# for PythonParser
reader.readline.side_effect = read
reader.readexactly.side_effect = read

async def open_connection(*args, **kwargs):
return reader, writer

with patch.object(asyncio, "open_connection", open_connection):
await conn.connect()

vals = await asyncio.gather(do_read(), do_close())
assert vals == [b"Hello, World!", None]


@pytest.mark.onlynoncluster
def test_create_single_connection_client_from_url():
client = Redis.from_url("redis://localhost:6379/0?", single_connection_client=True)
Expand Down
Loading
Loading