Add support for custom socket factories

[mina-asham]

This can enable UDS (Unix Domain Socket) or any other custom address resolution

• Related feature request: [NEW FEATURE] Support for SRV record #1998
• Related pull requests: #ref 1987 allowing user to define AddressResolver #2036 and Add Unix domain socket support #1942 and ADD UDS support #1132

[mina-asham]

@sazzad16 @gkorland can you have a look when you have time? This should unblock any custom connection

[sazzad16]

@mina-asham Please add a test or two. A test for UDS would be a great addition.

[mina-asham]

@sazzad16 I didn't want to add another dependency per our discussion in #1942, but thinking about it now, I think it makes sense to add the dependency as a test dependency, so will do that and port my test from there!

[mina-asham]

Test is failing because of spop test due to a fix in redis/unstable, see: #2152

[sazzad16]

Test is failing because of spop test due to a fix in redis/unstable

@mina-asham Don't worry about it. There are a couple of Redis commands that are in back-and-forth changes.

[sazzad16]

LGTM!

@mina-asham Not necessary, but can give a short explanation of change in SSLJedisClusterTest? (I'm just being lazy :P)

[mina-asham]

LGTM!

@mina-asham Not necessary, but can give a short explanation of change in SSLJedisClusterTest? (I'm just being lazy :P)

Ha, the main difference is that the previous code used to:

• Create and assign the socket field
• Then fail trying to wrap it in SSL (but at that point the socket field was already assigned, so the isConnected check would pass)

The new logic is all encapsulated in the factory, so you either get a socket or an exception, so the isConnected method would return false (which I believe is the right behavior), so the cluster failure is that there are no active nodes instead of trying to use a node it thinks is active but it's not

[mina-asham]

@gkorland do you have any comments on this one?

[gkorland]

@mina-asham it LGTM, but it's pretty hard to review the changes this way with the force-pushed. Next time please keep the incremental commits.

[mina-asham]

@mina-asham it LGTM, but it's pretty hard to review the changes this way with the force-pushed. Next time please keep the incremental commits.

Will do, thanks for reviewing!

[mina-asham]

Can we merge this one?

[sazzad16]

@mina-asham We will merge it, but there is a confusion about which version we'll be adding it in. That's all.

[gkorland]

I think we are missing here a solution for the JedisPool & JedisCluster

[mina-asham]

You're right, missed the respective constructors there, will add them and uodate this PR!

[sazzad16]

@gkorland I'm going ahead and merging this.
@mina-asham Please do further improvements in separate PR. Thanks for you contribution!