-
Notifications
You must be signed in to change notification settings - Fork 335
Conversation
Please fix pep8 errors and add tests |
Thanks for PR, |
@asvetlov @popravich Thanks for the feedback! I fixed the PEP8 whitespace issues. Regarding testing, I've only been using the It looks like when @oranagra of RedisLabs added SSL support to redis-py, he just noted that it worked with an SSL server. One option is to create a self-signed certificate and then use Any guidance on where I should go from here? I didn't know if you'd want to add something like stunnel to your testing environment. Thanks again for your consideration of this PR! P.S. Redislabs blog post when SSL was added to some popular clients. |
@@ -36,13 +36,14 @@ class RedisPool: | |||
"""Redis connections pool. | |||
""" | |||
|
|||
def __init__(self, address, db=0, password=None, encoding=None, | |||
def __init__(self, address, db=0, password=None, ssl=None, encoding=None, |
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 ssl
parameter here must be keyword-only argument.
Thank you for cleaning this up and merging it! I look forward to using it via |
I will do release this week. |
👍 That'd be great. Thanks again! |
By design, Redis doesn't natively support encryption via SSL/TLS. However, it can be added with an SSL proxy.
For this reason, some popular Redis libraries support SSL connections.
This change adds support for SSL/TLS by leveraging asyncio's existing SSL support in asyncio.BaseEventLoop.create_connection and using its semantics.
I didn't exhaustively research or test this change, so this pull request is simply my first pass at how such a change might look. I welcome critical feedback on it.
Thanks for this library!