-
Notifications
You must be signed in to change notification settings - Fork 68
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
Split cache config from queue config. #902
Conversation
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.
Just some documentation requests
cache: | ||
redisUri: "redis://redis-host:3679" | ||
``` | ||
|
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.
docs/advanced/encryption.md needs updating to mention that crypto requires this config to be enabled (instead of queue.monolithic
).
Co-authored-by: Andrew Ferrazzutti <andrewf@element.io> Signed-off-by: Will Hunt <will@half-shot.uk>
Signed-off-by: Will Hunt <will@half-shot.uk>
Signed-off-by: Will Hunt <will@half-shot.uk>
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.
The config in helm/hookshot/values.yaml
should be updated too.
Co-authored-by: Andrew Ferrazzutti <andrewf@element.io> Signed-off-by: Will Hunt <will@half-shot.uk>
Believe everything has been addressed 🥳 |
The reason these were conjoined is because we originally wanted a Redis for worker mode, and eventually started using it as a cache. These days we recommend using Redis as a cache, but not for message queueing. This is really confusing, and to be honest could easily be seperated into two configs.
This changes does just that. We're also going to move towards
redisUri
because frankly it allows more options for less work.