search

neumino / rethinkdbdash

An advanced Node.js driver for RethinkDB with a connection pool, support for streams etc.
MIT License
848 stars 109 forks source link

Reconnect using all options #350

Open aleclarson opened 7 years ago

aleclarson commented 7 years ago

Throw an error if options.connection is defined. Retain the options for subsequent calls to r.connect when trying to reconnect.

The following options were not being used in reconnections:

neumino commented 7 years ago

Left a few comments, it seems reasonable beside that

aleclarson commented 7 years ago

@neumino I can't find where you left the comments 😅

Extarys commented 7 years ago

@aleclarson I think he meant that you comment a little before the merge can happen.

Also, I noticed that there is a lot more options than the one being passed, like tls and connection pools, shouldn't we add those or it will reconnect with those settings by it's own?

aleclarson commented 7 years ago

@Extarys Not much to add comments to, it's just 3 changes and 1 addition. And there are already 2 comments.

The connection pool manages each connection. The connection has no knowledge of belonging to a connection pool. Thus, no pool-specific options exist in this context.

As for TLS, that configuration is done using the options.ssl object, which is already accounted for.

Extarys commented 7 years ago

@aleclarson That's why I wasn't sure if it was needed. I mean you guys are the pros I'm just a guy using the software :P Thanks for taking the time to clarify this for me, I feel more intelligent already

thelinuxlich commented 6 years ago

merged in https://github.com/RebirthDB/rebirthdb-js