#ref 1987 allowing user to define AddressResolver

[gkorland]

No description provided.

[sazzad16]

@gkorland Mostly LGTM! But a test would help users to understand how to use it.

[mina-asham]

Looks really close to what we reached in #1942, I was planning on adding a similar thing but for the whole socket. @gkorland would such a change be better, or do you think we need this one as well?

[sazzad16]

@mina-asham Compared to working on whole Socket, IMO, this PR is closer to both of our expectation.

I said that a custom object to create/connect a socket could be enough.

And you did not prefer to expose socket directly.

I think this PR sets a very good balance on both of our expectations and plans. So, for the time being, we can skip the idea of whole socket and support UDS based on this one. WDYT?

@gkorland ?

[mina-asham]

@mina-asham Compared to working on whole Socket, IMO, this PR is closer to both of our expectation.

I said that a custom object to create/connect a socket could be enough.

And you did not prefer to expose socket directly.

I think this PR sets a very good balance on both of our expectations and plans. So, for the time being, we can skip the idea of whole socket and support UDS based on this one. WDYT?

@gkorland ?

Happy to do it here, but I don't see how you can support UDS socket, without supporting some sort of socket factory here, am I missing something?

[sazzad16]

@mina-asham Before answering that, what were you planning to do in #1942 with whole socket? Was it with or without some socket factory?

[mina-asham]

@mina-asham Before answering that, what were you planning to do in #1942 with whole socket? Was it with or without some socket factory?

Apologies for the uber late reply, here is what I (roughly) intended to do:

• Similar to @gkorland commit here, allow passing a SocketFactory in the constructor and propagate as appropriate
• Extract this code into a DefaultSocketFactory

[sazzad16]

#2036 (comment)

Happy to do it here, but I don't see how you can support UDS socket, without supporting some sort of socket factory here, am I missing something?

I thought, by socket factory, you meant something like AFUNIXSocket. But it seems you actually meant something like DefaultSocketFactory. Isn't it?

[mina-asham]

#2036 (comment)

Happy to do it here, but I don't see how you can support UDS socket, without supporting some sort of socket factory here, am I missing something?

I thought, by socket factory, you meant something like AFUNIXSocket. But it seems you actually meant something like DefaultSocketFactory. Isn't it?

Yeah, the Connection class needs to be able to re-create the sockets when they fail, so we cannot supply it with the socket directly. The only way I can see it happening in that case is to supply a JedisSocketFactory interface that has a default implementation covering the current content of the Connection.connect method. This would enable any user to supply any kind of socket creation logic if they need to (e.g. a UDS socket)

[gkorland]

@mina-asham agree adding JedisSocketFactory can be the right solution here for both FRs.

[sazzad16]

@mina-asham Okay, I understand now. As I've mentioned in my previous comment, I was a bit misdirected.

And, apologies for late reply.

[sazzad16]

@gkorland @mina-asham any update?

[mina-asham]

@sazzad16 I was under the impression that we are abandoning my PR for @gkorland's here. @gkorland tell me if you want me to pick it up instead of you?

[sazzad16]

Resolved by #2151