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
settings for redis sentinel #3266
settings for redis sentinel #3266
Conversation
src/ralph/settings/base.py
Outdated
REDIS_MASTER_IP = None | ||
REDIS_MASTER_PORT = None | ||
|
||
REDIS_SENTINEL_ENABLED = os.environ.get('REDIS_SENTINEL_ENABLED', 'false').lower() == 'true' # noqa: E501 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use str_to_bool
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
src/ralph/settings/base.py
Outdated
REDIS_CONNECTION = { | ||
'HOST': os.environ.get('REDIS_HOST', 'localhost'), | ||
'PORT': os.environ.get('REDIS_PORT', '6379'), | ||
'HOST': REDIS_MASTER_IP if REDIS_MASTER_IP else os.environ.get('REDIS_HOST', 'localhost'), # noqa: E501 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not REDIS_MASTER_IP or os.environ.get('REDIS_HOST', 'localhost')
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right, or
looks better
src/ralph/settings/base.py
Outdated
|
||
sentinel = Sentinel( | ||
[tuple(s_host.split(':')) for s_host in SENTINEL_HOSTS], | ||
socket_timeout=0.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be better allow to parameterize this timeout as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/ralph/settings/base.py
Outdated
from redis.sentinel import Sentinel | ||
|
||
# SENTINEL_HOSTS env variable format: host_1:port;host_2:port | ||
SENTINEL_HOSTS = os.environ['SENTINEL_HOSTS'].split(';') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider naming it REDIS_SENTINEL_HOSTS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no concerns left. LGTM, thanks!
No description provided.