Add SSL option for redis client #5
Conversation
|
@tillepille I'm actually not involved in this redis plugin. your changes look good though. maybe @masom has a spare minute to look at this? 👋 |
|
@tillepille i'm not sure how many active maintainers there are for the thumbor-community plugins. would you be interested in helping out? |
|
@phoet I think I'm not the best choice, since I'm not a python enthusiast(,yet) nor someone who writes code a lot... |
|
Ditto, though I changed jobs so I'm not dealing with python or using thumbor any longer... @masom is probably busy at Shopify at the moment. I'll try to reach out to him if he does not react to this within the next couple of days. I think there was some slack-group for this, but I can't find it right now... |
There was a problem hiding this comment.
Thank you for contributing this patch to tc_redis.
Would it be possible to add a test in https://github.com/thumbor-community/redis/blob/master/vows/redis_storage_vows.py to ensure the new option is correctly working?
|
@masom Since this is a flag which is present in the underlying lib, do you want to test the functionality of that lib? |
|
I think testing the functionality of the lib is a tiny bit out of scope although we should ensure we are indeed passing valid options to the lib (so it doesn't raise on init). The test itself could just be as simple as connecting with SSL enabled, and checking that we are indeed connected with SSL. Looking at the redis-py docs, there's also a few more SSL options, should those also be implemented? |
|
@masom |
closes #4