Skip to content

Conversation

@aleclarson
Copy link
Contributor

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:

  • username
  • password
  • pingInterval
  • timeout
  • ssl

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:
  - username
  - password
  - pingInterval
  - timeout
  - ssl
@neumino
Copy link
Owner

neumino commented Sep 3, 2017

Left a few comments, it seems reasonable beside that

@aleclarson
Copy link
Contributor Author

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

@Extarys
Copy link

Extarys commented Sep 12, 2017

@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
Copy link
Contributor Author

aleclarson commented Sep 12, 2017

@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
Copy link

Extarys commented Sep 13, 2017

@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
Copy link

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.

4 participants