Skip to content

allow tls as scheme in DSN string for predis library #515

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

Closed
wants to merge 4 commits into from

Conversation

jeff1985
Copy link
Contributor

Predis allows connecting to TLS encrypted endpoints by using scheme=tls which is not allowed by current enqueue library. This is needed to connect to major cloud provider's redis services. I added the posibility to specify tls:// endpoints in DSN string for each configuration.

See predis docs for more info on TLS: https://github.com/nrk/predis

My DSN string looks as following:

ENQUEUE_DSN=tls://xxx.redis.cache.windows.net:6380?password=yyyy&ssl[verify_peer]=1&persistent=1&vendor=predis

@makasim
Copy link
Member

makasim commented Aug 24, 2018

Tls is to broad, I’d suggest changing it to scheme redis+tls

@jeff1985
Copy link
Contributor Author

Hi @makasim ,

just tested: redis+tls throws exception in Predis library:

Unknown connection scheme: 'redis+tls'

another possibility would be rediss which is alias of tls. I just tested and it just works fine.

Agree?

@makasim
Copy link
Member

makasim commented Aug 24, 2018

Agree

@jeff1985
Copy link
Contributor Author

OK @makasim, I changed it to rediss.

Please accept the MR!

@makasim
Copy link
Member

makasim commented Aug 24, 2018

Could you please add some tests to RedisConnectionFactoryConfigTest?

@jeff1985
Copy link
Contributor Author

Hi @makasim,

I added tests to RedisConnectionFactoryConfigTest as you requested!

@jeff1985
Copy link
Contributor Author

jeff1985 commented Sep 6, 2018

Hi @makasim

were you able to finish your review?

@makasim makasim changed the base branch from 0.9 to master September 7, 2018 11:42
@makasim makasim closed this Sep 7, 2018
@jeff1985
Copy link
Contributor Author

jeff1985 commented Sep 7, 2018

Thanks for that!

@makasim makasim added this to the 0.9 milestone Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants