rq / django-rq

A simple app that provides django integration for RQ (Redis Queue)
MIT License
1.81k stars 282 forks source link

Sentinel support broken since 2.9.0 #642

Open moonrail opened 6 months ago

moonrail commented 6 months ago

Hello altogether,

while using redis sentinel we've run into the following exception with django-rq 2.9.0 and 2.10.1:

Traceback (most recent call last):
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/views/decorators/cache.py", line 62, in _wrapper_view_func
    response = view_func(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/contrib/auth/decorators.py", line 23, in _wrapper_view
    return view_func(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_rq/views.py", line 37, in stats
    **get_scheduler_statistics(),
      ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_rq/utils.py", line 106, in get_scheduler_statistics
    conn_key = f"{connection_kwargs['host']}:{connection_kwargs['port']}/{connection_kwargs.get('db', 0)}"
                  ^^^^^^^^^^^^^^^^^^^^^^^^^

Exception Type: KeyError at /admin/background-tasks/
Exception Value: 'host'

Sentinel configuration does not have a host key, hence the KeyError.

The cause can be found here: https://github.com/rq/django-rq/blob/v2.10.1/django_rq/utils.py#L106

It was introduced with this commit: https://github.com/rq/django-rq/commit/df3271341a3668717844dcdc9a4a77e64a6316d0

I like that tests were added to test this new functionality as well, but they should also cover sentinel setups, as this library supports them and even provides example configuration as simple in the README.md for it.

We've stumbled over this problem via usage of NetBox, a Django based application leveraging django-rq: https://github.com/netbox-community/netbox/issues/14752

terencehonles commented 6 months ago

This appears to depend on how you have your sentinel configured. We use Django's CACHES to define the redis connection via django-redis instead of django-rq since it was more expressive (at least at the time) and it will have a host, but not a port as I fixed in https://github.com/rq/django-rq/pull/637.

You may want to provide a minimal config to either easily reproduce or allow someone to create an appropriate test. If you have the time a PR would be helpful :slightly_smiling_face:

moonrail commented 6 months ago

@terencehonles As referenced in the description of this issue, the minimal config from README.md of django-rq:

RQ_QUEUES = {
    'default': {
        'HOST': 'localhost',
        'PORT': 6379,
        'DB': 0,
        'USERNAME': 'some-user',
        'PASSWORD': 'some-password',
        'DEFAULT_TIMEOUT': 360,
        'REDIS_CLIENT_KWARGS': {    # Eventual additional Redis connection arguments
            'ssl_cert_reqs': None,
        },
    },
    'with-sentinel': {
        'SENTINELS': [('localhost', 26736), ('localhost', 26737)],
        'MASTER_NAME': 'redismaster',
        'DB': 0,
        # Redis username/password
        'USERNAME': 'redis-user',
        'PASSWORD': 'secret',
        'SOCKET_TIMEOUT': 0.3,
        'CONNECTION_KWARGS': {  # Eventual additional Redis connection arguments
            'ssl': True
        },
        'SENTINEL_KWARGS': {    # Eventual Sentinel connection arguments
            # If Sentinel also has auth, username/password can be passed here
            'username': 'sentinel-user',
            'password': 'secret',
        },
    },
    'high': {
        'URL': os.getenv('REDISTOGO_URL', 'redis://localhost:6379/0'), # If you're on Heroku
        'DEFAULT_TIMEOUT': 500,
    },
    'low': {
        'HOST': 'localhost',
        'PORT': 6379,
        'DB': 0,
    }
}

As you can see: host is nowhere being set for with-sentinel. As to why a KeyError on accessing the Connection Kwargs dictionary directly is more than plausible.

To provide more example config, take a look at referenced NetBox issue description: https://github.com/netbox-community/netbox/issues/14752

selwin commented 4 months ago

Hi, sentinel support should be added to RQ itself so please open an issue there.

moonrail commented 4 months ago

@selwin rq already supports Sentinel: https://python-rq.org/docs/connections/#sentinel-support

Also django-rq has a Sentinel config-example in the README.md: https://github.com/rq/django-rq/blob/v2.10.1/README.rst?plain=1#L60-L76

selwin commented 4 months ago

@moonrail PR welcome :)