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

TypeError: __init__() got an unexpected keyword argument 'connection_pool' #663

Open
sohamM97 opened this issue Apr 5, 2023 · 2 comments
Labels

Comments

@sohamM97
Copy link

sohamM97 commented Apr 5, 2023

Describe the bug
We are trying to use django redis cache with sentinel and SSL connection in our application. The issue occurs when we use rediss as part of the URL given in the LOCATION setting, as described below.

To Reproduce

Keep the CACHES setting on settings.py as follows:

# there our other settings which we take from .env too, but mentioning this here for clarity
REDIS_SSL = env("REDIS_SSL", bool, default=False)
# in our case, since REDIS_SSL is true, REDIS_PROTOCOL would be 'rediss'
REDIS_PROTOCOL = "rediss" if REDIS_SSL else "redis"

CACHES = {
    "default": {
        "BACKEND": "django_redis.cache.RedisCache",
        # The next line is where the issue occurs since we use REDIS_PROTOCOL 
        # which is 'rediss' in this case.
        "LOCATION": f"{REDIS_PROTOCOL}://{REDIS_MASTER}/{REDIS_DB}",
        "OPTIONS": {
            "CLIENT_CLASS": "django_redis.client.SentinelClient",
            "SENTINELS": SENTINELS,
            "SENTINEL_KWARGS": {
                "password": SENTINEL_PASSWORD,
                "ssl_certfile": REDIS_SSL_CERTFILE,
                "ssl_keyfile": REDIS_SSL_KEYFILE,
                "ssl_ca_certs": REDIS_SSL_CA_CERTS,
                "ssl_cert_reqs": None,
                "ssl": REDIS_SSL,
            },
            "SOCKET_TIMEOUT": SENTINEL_CONNECTION_TIMEOUT,
            "SOCKET_CONNECT_TIMEOUT": SENTINEL_CONNECTION_TIMEOUT,
            "CONNECTION_POOL_CLASS": "redis.sentinel.SentinelConnectionPool",
            "PASSWORD": REDIS_PASSWORD,
            "CONNECTION_POOL_KWARGS": {
                "ssl": REDIS_SSL,
                **(
                    {
                        "ssl_certfile": REDIS_SSL_CERTFILE,
                        "ssl_keyfile": REDIS_SSL_KEYFILE,
                        "ssl_ca_certs": REDIS_SSL_CA_CERTS,
                        "ssl_cert_reqs": None,
                    }
                    if REDIS_SSL
                    else {}
                ),
            },
        },
        "KEY_PREFIX": "cache",
        "TIMEOUT": CACHE_TIMEOUT,
    },
}

Note that we take the settings such as REDIS_MASTER, REDIS_DB etc from our environment file (.env), which we read using the django-environ library.

Expected behavior
The library should be able to connect to the sentinel-based setup and the application should start up properly on running the server.

Stack trace

Traceback (most recent call last):
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/connection.py", line 1435, in get_connection
    connection = self._available_connections.pop()
IndexError: pop from empty list

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/soham/aistudio/aeli_utils/cache.py", line 47, in get_from_cache
    return cache.get(key, default)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/django_redis/cache.py", line 91, in get
    value = self._get(key, default, version, client)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/django_redis/cache.py", line 31, in _decorator
    return method(self, *args, **kwargs)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/django_redis/cache.py", line 98, in _get
    return self.client.get(key, default=default, version=version, client=client)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/django_redis/client/default.py", line 258, in get
    value = client.get(key)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/commands/core.py", line 1790, in get
    return self.execute_command("GET", name)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/client.py", line 1255, in execute_command
    conn = self.connection or pool.get_connection(command_name, **options)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/connection.py", line 1437, in get_connection
    connection = self.make_connection()
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/connection.py", line 1479, in make_connection
    def release(self, connection):
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/connection.py", line 1085, in __init__
    super().__init__(**kwargs)
  File "/home/soham/envs/aistudio/lib/python3.9/site-packages/redis/connection.py", line 941, in __init__
    super().__init__(**kwargs)
TypeError: __init__() got an unexpected keyword argument 'connection_pool'

Environment (please complete the following information):

  • Python version: 3.9
  • Django Redis Version: 5.0.0
  • Django Version: 3.2.6
  • Redis Version: 7.0.10
  • redis-py Version: 4.5.4

Additional context
Instead of keeping the LOCATION as f"{REDIS_PROTOCOL}://{REDIS_MASTER}/{REDIS_DB}", if we keep it as f"redis://{REDIS_MASTER}/{REDIS_DB}", the application works as expected. We are confused as to why we need to keep the protocol as redis even though we clearly want to use rediss.

@sohamM97 sohamM97 added the bug label Apr 5, 2023
@grahamneville
Copy link

I'm also hitting this issue.
Are there any updates as to why this might be occurring?

@chunechew
Copy link

chunechew commented Aug 14, 2024

@sohamM97 @grahamneville

I've written a temporary monkey-patching codes for fixing this bug:

The temporary code (e.g. path/to/sentinel.py):

from urllib.parse import urlparse
from django.core.exceptions import ImproperlyConfigured
from django.utils.module_loading import import_string
from django_redis.pool import ConnectionFactory


class CustomSentinelConnectionFactory(ConnectionFactory):
    def __init__(self, options):
        # allow overriding the default SentinelConnectionPool class
        pool_cls_path = options.setdefault(
            "CONNECTION_POOL_CLASS", "redis.sentinel.SentinelConnectionPool"
        )
        self.pool_cls = import_string(pool_cls_path)
        self.pool_cls_kwargs = options.get("CONNECTION_POOL_KWARGS", {})

        sentinel_cls_path = options.get("SENTINEL_CLASS", "redis.sentinel.Sentinel")
        self.sentinel_cls = import_string(sentinel_cls_path)

        redis_client_cls_path = options.get("REDIS_CLIENT_CLASS", "redis.client.Redis")
        self.redis_client_cls = import_string(redis_client_cls_path)

        connection_pool_cls_path = options.get(
            "CONNECTION_POOL_CLASS", "redis.sentinel.SentinelConnectionPool"
        )
        self.connection_pool_cls = import_string(connection_pool_cls_path)

        self.redis_client_cls_kwargs = options.get("REDIS_CLIENT_KWARGS", {})

        self.options = options

        sentinels = options.get("SENTINELS")
        if not sentinels:
            raise ImproperlyConfigured(
                "SENTINELS must be provided as a list of (host, port)."
            )

        self.min_other_sentinels = options.get("MIN_OTHER_SENTINELS", 0)
        self.sentinel_kwargs = options.get("SENTINEL_KWARGS", {})

        # provide the connection pool kwargs to the sentinel in case it
        # needs to use the socket options for the sentinels themselves
        connection_kwargs = self.make_connection_params(None)
        connection_kwargs.pop("url")
        connection_kwargs.update(self.pool_cls_kwargs)
        self.connection_kwargs = connection_kwargs
        self._sentinel = self.sentinel_cls(
            sentinels,
            self.min_other_sentinels,
            self.sentinel_kwargs,
            **self.connection_kwargs,
        )

    def get_connection_pool(self, params):
        url = urlparse(params["url"])
        master_name = url.hostname

        if master_name is None:
            raise ValueError(
                'SENTINEL_SETTINGS["master_name"] must be specified in the Django settings.'
            )

        pool = self.connection_pool_cls(
            master_name, self._sentinel, **self.connection_kwargs
        )

        return pool

An example of the Django settings file (settings.py):

import ssl

# ...

REDIS_SSL_SETTINGS = {
    "ssl_ca_certs": "relative/path/to/redis_ssl_ca_certs.crt",   # Modify this to the correct file name and path
    "ssl_certfile": "relative/path/to/redis_ssl_certfile.crt",   # Modify this to the correct file name and path
    "ssl_keyfile": "relative/path/to/redis_ssl_keyfile.key",  # Modify this to the correct file name and path
}

DJANGO_REDIS_CONNECTION_FACTORY = "path.to.sentinel.CustomSentinelConnectionFactory"  # Modify this to the correct file name and path
SENTINEL_HOST_NAME = "sentinel.host.name" # Modify this to the correct Sentinel host name
SENTINEL_PORT = 26379  # Modify this to the correct Sentinel port number
SENTINELS = [
    (SENTINEL_HOST_NAME , SENTINEL_PORT),
]
REDIS_PASSWORD = "password"  # Modify this to the correct password
SENTINEL_MASTER_NAME = "mymaster"  # Modify this to the correct name of the master
REDIS_PORT = 6379  # Modify this to the correct port number of the master and the slaves
REDIS_DB = 0  # Modify this to the correct DB number of the master and the slaves

SENTINEL_KWARGS = {
    "password": REDIS_PASSWORD,
    "ssl": True,
    "ssl_cert_reqs": ssl.CERT_REQUIRED,
    **REDIS_SSL_SETTINGS,
}
SENTINEL_SETTINGS = {
    "master_name": SENTINEL_MASTER_NAME,
    "sentinel_kwargs": SENTINEL_KWARGS,
}
REDIS_SENTINEL_COMMON_KWARGS = {   # You may change or add the values
    "max_connections": 100,
    "ssl": True,  # Don't change or remove this
    "db": REDIS_DB 
    "port": REDIS_PORT,
    "ssl_cert_reqs": ssl.CERT_REQUIRED,
    **REDIS_SSL_SETTINGS,  # Don't change or remove this
}

CACHES = {
    "default": {
        "BACKEND": "django_redis.cache.RedisCache",
        "LOCATION": f"rediss://{SENTINEL_MASTER_NAME}/{REDIS_DB}",
        "OPTIONS": {
            "CLIENT_CLASS": "django_redis.client.SentinelClient",
            "CONNECTION_POOL_KWARGS": REDIS_SENTINEL_COMMON_KWARGS,
            "REDIS_CLIENT_KWARGS": REDIS_SENTINEL_COMMON_KWARGS,
            "PASSWORD": REDIS_PASSWORD,
            "SENTINELS": SENTINELS,
            "SENTINEL_KWARGS": SENTINEL_KWARGS,
            "CONNECTION_POOL_CLASS": "redis.sentinel.SentinelConnectionPool",
        },
    }
}

# ...

Until the Django-Redis dev team fixes the bug, we can avoid it using these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants