Use symbols everywhere for database configurations #37185
Conversation
seejohnrun
force-pushed
the
config-symbols
branch
14 times, most recently
from
99943e7
to
0927160
Compare
seejohnrun
force-pushed
the
config-symbols
branch
from
0927160
to
df733bc
Compare
| @@ -13,6 +13,11 @@ def initialize(env_name, spec_name) | |||
| @spec_name = spec_name | |||
| end | |||
|
|
|||
| def config | |||
| ActiveSupport::Deprecation.warn("DatabaseConfig#config will be removed in favor of DatabaseConfigurations#configuration_hash which returns a hash with symbol keys") | |||
There was a problem hiding this comment.
We should update this to say what version it will be removed in which I guess since this is going into 6.1.0 it should be removed in 6.2.0
Previously in some places we used symbol keys, and in some places we used string keys. That made it pretty confusing to figure out in a particular place what type of configuration object you were working with. Now internally, all configuration hashes are keyed by symbols and converted to such on the way in. A few exceptions: - `DatabaseConfigurations#to_h` still returns strings for backward compatibility - Same for `legacy_hash` - `default_hash` previously could return strings, but the associated comment mentions it returns symbol-key `Hash` and now it always does Because this is a change in behavior, a few method renames have happened: - `DatabaseConfig#config` is now `DatabaseConfig#configuration_hash` and returns a symbol-key `Hash` - `ConnectionSpecification#config` is now `ConnectionSpecification#underlying_configuration_hash` and returns the `Hash` of the underlying `DatabaseConfig` - `DatabaseConfig#config` was added back, returns `String`-keys for backward compatibility, and is deprecated in favor of the new `configuration_hash` Co-authored-by: eileencodes <eileencodes@gmail.com>
seejohnrun
force-pushed
the
config-symbols
branch
from
df733bc
to
ce9b197
Compare
|
|
| @@ -13,6 +13,11 @@ def initialize(env_name, spec_name) | |||
| @spec_name = spec_name | |||
| end | |||
|
|
|||
| def config | |||
There was a problem hiding this comment.
This object is private API. Do we need to deprecate and have a Changelog entry?
There was a problem hiding this comment.
This object is private but the other two HashConfig and UrlConfig which inherit from this aren't. They shouldn't be used directly but when you loop through the configurations it will call config and 💥
Some apps may be looping through the configs and then calling config on it. I know we are in GitHub - and eventually all that code will go away as we remove the multi-db stuff but it's not impossible someone is relying on it.
ActiveRecord::Base.configurations.configs_for(env_name: "development").each do |db_config|
db_config.config # this would blow up
endThere was a problem hiding this comment.
Oops I totally wrote the wrong thing here. I'm going back and forth between our app and Rails work and I dunno.
The reason I want to deprecate the string access is because I know at least our app depends on it. It's likely apps are doing db_config.config["database"] because they used to access that with a hash. I'm more comfortable deprecating than potentially breaking apps and gems that rely on the old hash behavior. Especially as we move away from hashes internally (and maybe one day in apps themselves).
Calling `db_config.config` was deprecated in rails#37185 in favor of `db_config.configuration_hash`. When returning the objects from `configurations` we should ensure the object returns the non-deprecated method. Before: ``` <ActiveRecord::DatabaseConfigurations::HashConfig:0x00007ff0f61696a0 @env_name="development", @name="primary", @config={:adapter=>"mysql2", :database=>"recipes_development"}> ``` After: ``` <ActiveRecord::DatabaseConfigurations::HashConfig:0x00007ff0f61696a0 @env_name="development", @name="primary", @configuration_hash={:adapter=>"mysql2", :database=>"recipes_development"}> ``` Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Previously in some places we used symbol keys, and in some places we used
string keys. That made it pretty confusing to figure out in a particular
place what type of configuration object you were working with.
Now internally, all configuration hashes are keyed by symbols and
converted to such on the way in.
A few exceptions:
DatabaseConfigurations#to_hstill returns strings for backward compatibilitylegacy_hashdefault_hashpreviously could return strings, but the associatedcomment mentions it returns symbol-key
Hashand now it always doesBecause this is a change in behavior, a few method renames have happened:
DatabaseConfig#configis nowDatabaseConfig#configuration_hashand returns a symbol-keyHashConnectionSpecification#configis nowConnectionSpecification#underlying_configuration_hashand returns theHashof the underlyingDatabaseConfigDatabaseConfig#configwas added back, returnsString-keys for backward compatibility, and is deprecated in favor of the newconfiguration_hashcc / @eileencodes