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

feat: allow setting Redis connection options #304

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexei
Copy link

@alexei alexei commented Jul 13, 2021

redis.from_url may accept connection options specified as kwargs. They're not particularly documented, but a list may be found in the source code:

URL_QUERY_ARGUMENT_PARSERS = {
    'db': int,
    'socket_timeout': float,
    'socket_connect_timeout': float,
    'socket_keepalive': to_bool,
    'retry_on_timeout': to_bool,
    'max_connections': int,
    'health_check_interval': int,
    'ssl_check_hostname': to_bool,
}

I think most people don't need them and django-health-check works fine with the defaults.

That said, there is one particular case which I think django-health-check needs to deal with which is e.g. Heroku uses self signed certificates which causes the connection to fail (raising a ConnectionError) which in turn makes the Redis health check to report the service as unavailable. The issue can be worked around by disabling SSL verification. In order to do that, one needs to pass ssl_cert_reqs=None to the connection object.

This PR introduces a new variable HEALTHCHECK_REDIS_URL_OPTIONS which allows one to customise the connection if they need to.

@brockhaywood
Copy link

+1

@onlinejudge95
Copy link

@KristianOellegaard can we allow this, we use Redis with SSL in our cluster and our Django startupProbe are failing due to this

@BoPeng
Copy link

BoPeng commented Apr 24, 2023

Ditto. Our healthcheck also fails to connect to rediss://.

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

Successfully merging this pull request may close these issues.

4 participants