Remove sensitive options before calling after_connect #336
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #335
The issue is that all of the connection options are passed to
after_connect. And if that fails for some reason it can be logged out in the stack trace.There are a few ways that this could have been handled. I'll explain them briefly and why I went with the current approach:
show_sensitive_connection_data...option apply toafter_connect. This didn't seem appropriate because there is nothing inherently sensitive aboutafter_connect. It could be any random query.after_connect. This felt a bit risky to me because I don't know what can/will be useful in the future from all of the repo options we currently let people set or will let them set in the future.after_connect. The risk here is we miss certain things or forget to update the list as new sensitive options are added.I chose option 3 because it seemed the least likely to cause regressions. It doesn't seem super likely that we will add a lot of new sensitive options to the connection.
I'm on the fence about changing it to option 2 though because it might be obvious to someone else what the inclusion list should be.