Transform options keys to symbols.

[joshuapinter]

Before this, any options with a String key (e.g. "host") instead of a Symbol key (e.g. :host) would get silently ignored.

This is surprising in general but in particular, when using Rails and reading the database.yml config, they are all String keys so have no affect unless they are transformed before passing to Trilogy.new.

This reduces the surprises and ensures the keys are all transformed to Symbols before accessing them.

Fixes #149.

[brianmario]

I would have expected Rails to be using HashWithIndifferentAccess here, strange...

[brianmario]

I may be off track here, but I think this is where ActiveRecord is symbolizing the keys of the config hash before then being passed to and used by the underlying driver.

In any case, I think this is worth adding to the codebase 👍

[jhawthorn]

I'm not sure about this. We shouldn't expect to be able to pass the hash Rails uses for configuration into Trilogy.new, they happen to be almost identical but aren't compatible. For example the :ssl_mode key in Rails is different https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/trilogy_adapter.rb#L32-L33.

For other Rails adapters this is even more true and the keys are not the same https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L310-L320

I'm on the fence because this is probably fine but it might be better not to accept string keys and leave the option open for us to warn or error on invalid configuration in the future.

[joshuapinter]

I think either transforming the keys to symbols or showing a warning would do the trick.

The silently ignoring them was problematic for us because this "worked" in Development:

config = Rails.configuration.database_configuration[ Rails.env ]
connection = Trilogy.new( config )
#=> <Trilogy:0x000000013a9de1e0

But in reality, the keys were just completely ignored and it wasn't until we got to Staging and Production that we saw an error like this:

config = Rails.configuration.database_configuration[ Rails.env ]
connection = Trilogy.new( config )
#=> .../trilogy-2.6.1/lib/trilogy.rb:18:in `_connect': No such file or directory - trilogy_connect - unable to connect to /tmp/mysql.sock (Trilogy::SyscallError::ENOENT)

We know about it now but doing one of these options would help somebody in the future getting caught off guard.

[eileencodes]

I'm curious @joshuapinter, is there a reason are you connecting to the db with the config hash from Railties instead of Active Record? AR does a bunch of parsing to make the config from Railties valid, and with the introduction of db config objects, accessing the hash is discouraged.

I think the connection will work fine if you use ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).first instead of Rails.configuration.database_configuration[Rails.env]?

[joshuapinter]

Hi @eileencodes, we're connecting to a secondary, temporary database during a migration/conversion to pull data from the secondary database over to our primary database/app. It's in the same MySQL server with the same credentials but just a different database name.

So we pull the config for our primary database and then just change the database name and pass that config to Trilogy to return a connection we can write queries against.

This was the best way I found to do this but not sure if there's better ways.

[eileencodes]

So we pull the config for our primary database and then just change the database name and pass that config to Trilogy to return a connection we can write queries against.

I would recommend adding another entry to the datatabase yaml that inherits the values from the primary and changes just the db name. But I'm still not sure why you would need the Railties version, you can access the config hash on the object with ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).first.configuration_hash.

[joshuapinter]

Thanks for the recommendation but database name is generated dynamically as part of the import/migration process.

That being said, I didn't know ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).first.configuration_hash was a thing and it worked great.

The only thing I needed to do was .dup before passing it to Trilogy because it's frozen and I get a can't modify frozen Hash error otherwise.

Do you want me to close this PR?

[jhawthorn]

Closing this. In the future we may warn or error, but it should be easy enough for callers to use symbols and Rails will also do this for us.