Add support for lookup parameter (#1) #2564
Conversation
* add lookup support to ssl param The docs @ https://node-postgres.com/api/client claim that the ssl param is "passed directly to node.TLSSocket, supports all tls.connect options". That statement is inaccurate, this patch doesn't make it fully compatible with all tls.connect options, but it does add support for the lookup parameter that's ubiquitous across all node connect functions.
|
Alternatively maybe the documentation should change to claim it's passed directly to "createSecureContext" and there should be a separate param for connect options |
There was a problem hiding this comment.
I think it should just go in the regular options, since it applies to non-SSL connections as well. (The documentation change suggestion seems good.)
Also, if you update it to do that, no need to include the date/change comment; there’s nothing particularly unusual going on and that’s what version control is for.
(A feature should also have a test before it goes in, but you don’t have to be the one to write it.)
|
Not ignoring your change request, planning on implementing it, time is just a scarce commodity for me at the moment. |
I feel that. no worries. |
The docs @ https://node-postgres.com/api/client claim that the ssl param is "passed directly to node.TLSSocket, supports all tls.connect options". That statement is inaccurate as "connect" is only called with port and host. This patch doesn't delve into trying to make it fully compatible with all tls.connect options, but it does add support for the lookup parameter that's ubiquitous across all node connect functions.