search

IlyaSkriblovsky / txredisapi

non-blocking redis client for python twisted
Apache License 2.0
235 stars 91 forks source link

Add ssl connector #147

Closed abhinavkulkarni closed 1 year ago

abhinavkulkarni commented 3 years ago

This PR adds SSL functionality for various networked connection methods. These days, most of the cloud providers offer SSL connections to managed Redis services hence this change is needed.

Only significant change is in makeConnection method. By default use_ssl=False, so the previous code should work as is.

mromejko commented 2 years ago

@IlyaSkriblovsky why don't you merge these changes? :)

IlyaSkriblovsky commented 2 years ago

@mromejko Good question! Because:

  1. There is unaddressed issue with variable names in the current version of the PR
  2. There is obvious way to make it better and more general (see my suggestion above about ssl argument to be of ClientContextFactory | bool | None type).
  3. I'm not sure anybody tested it in the real life. At least I didn't (I don't use ssl with txredisapi in production) and @abhinavkulkarni didn't posted anything about it :(

If you are interested in getting this merged, could you please check suggestions above about ssl/use_ssl mixed up in the code and test that it works?

mromejko commented 2 years ago

@IlyaSkriblovsky so if you aren't using ssl with txredisapi then what are you using? I will check you suggestions.

IlyaSkriblovsky commented 2 years ago

@mromejko I'm using it inside safeguarded context without any external access. Ok, I agree that I should have been using SSL even in this context :)

roeltm commented 1 year ago

Continued here: #148