django-rq icon indicating copy to clipboard operation
django-rq copied to clipboard

Sentinel support broken since 2.9.0

Open moonrail opened this issue 1 year ago • 5 comments
trafficstars

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

moonrail avatar Jan 16 '24 15:01 moonrail

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:

terencehonles avatar Jan 18 '24 09:01 terencehonles

@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

moonrail avatar Jan 18 '24 10:01 moonrail

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

selwin avatar Mar 03 '24 13:03 selwin

@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

moonrail avatar Mar 04 '24 09:03 moonrail

@moonrail PR welcome :)

selwin avatar Mar 04 '24 11:03 selwin