-
Notifications
You must be signed in to change notification settings - Fork 82
Print warning when unknown options are passed to Trilogy#initialize #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
contrib/ruby/lib/trilogy.rb
Outdated
| end | ||
| $stderr.puts "WARNING: Unknown Trilogy options: #{original_options.keys.join(", ")}" unless original_options.empty? | ||
| invalid_keys = options.keys - VALID_OPTIONS | ||
| $stderr.puts "WARNING: Unknown Trilogy options: #{invalid_keys.keys.join(", ")}" unless original_options.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Rails might pass through the whole db config, which includes keys that wouldn't be valid here. We should check that before merging this.
|
|
||
| class Trilogy | ||
| VALID_OPTIONS = %i[ | ||
| host port path database username password encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the distinction between socket and path options?
I see this:
trilogy/contrib/ruby/script/benchmark
Lines 15 to 16 in 362ee58
| path: DEFAULT_SOCK, # for trilogy | |
| socket: DEFAULT_SOCK, # for mysql2 |
..but also this:
trilogy/contrib/ruby/test/test_helper.rb
Line 61 in 362ee58
| socket: socket, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think path is a valid option for either trilogy or mysql2. Maybe it used to be called path?
Alternative to #151.
I'm not confident I have caught all of the possible configuration options.