Add support ssl connections to redis #3848
Conversation
| @@ -30,6 +31,12 @@ | |||
| CELERY_RESULT_EXPIRES = int(os.environ.get( | |||
| "REDASH_CELERY_RESULT_EXPIRES", | |||
| os.environ.get("REDASH_CELERY_TASK_RESULT_EXPIRES", 3600 * 4))) | |||
| CELERY_SSL_CONFIG = { | |||
There was a problem hiding this comment.
@rauchy thank you for the feedback! How does this look as far as settings now?
I'll try this change out on our self-hosted setup later today
There was a problem hiding this comment.
Settings look fine to me. However, the build is failing and requires your attention.
Co-Authored-By: Omer Lachish <omer@rauchy.net>
|
Thank you for opening this Pull Request, @nason. Based on the failing tests, it seems that the new code does not work when using no SSL configuration. Can you please check this out? |
|
On it! I saw the failing tests once I made this configurable but wasn't sure how to read them. I will look into this later today and update. |
|
I’ve updated with config changes to ensure that ssl config is not passed to celery unless ssl is enabled. The build and tests seem ok again, and our self hosted deploys with this branch and SSL enabled Redis is still working correctly |
|
👍 thanks! |
* Add support ssl connections to redis * Fix line length * Update redash/__init__.py w suggestion Co-Authored-By: Omer Lachish <omer@rauchy.net> * Cleanup init after suggestion * Move redis SSL config to settings * Do not pass celery SSL config unless necessary * Fix typo
What type of PR is this? (check all applicable)
Description
Allows TLS connection to Redis via
redissscheme https://www.iana.org/assignments/uri-schemes/prov/redissThis is achieved mostly via dependency upgrades, and some config tweaks. I've been running this on our self-hosted for a few weeks and its been working great.
Related Tickets & Documents
This follows #2357
I'd appreciate any thoughts or pointers on:
ssl_configbe passed in?