-
Notifications
You must be signed in to change notification settings - Fork 127
Fixes #727 - Add Redis Sentinel environment variables #734
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
Conversation
ArtifactsProduced during runtime |
|
@mgruner I tried to include it in the environment variables page but decided against it. Now there is a new page in the "Appendix" section. Additionally, I slightly rephrased the Redis section in the prerequisites and added a sentence to the already existing Just one thing I noticed: the environment variables page says Feel free to even suggest a complete rework... I could not really find additional information which could be useful for admins... |
|
Looks good to me!
Yes, I also believe that credentials can be passed in via the URL. You can try this locally with a standard Redis setup. Maybe you can get it to fail when supplying username/password if it is not needed by the redis service, maybe you have to set up the redis service with authentication first. Would be good to unify this, and remove this restriction. |
mgruner
left a comment
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.
Looks good! Consider removing the username/password discrepancy/restriction.
|
@mgruner Thanks! I tested it and after some hassle it worked in the docker compose stack. So I removed the part that no authentication is possible. |
No description provided.