1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Commit graph

12 commits

Author SHA1 Message Date
Rafael Mendonça França
4692cdda5f
Remove deprecated methods from ActiveRecord::DatabaseConfigurations 2020-10-30 00:25:37 +00:00
Ryuta Kamizono
92ff708476 Re-enable Layout/SpaceAroundOperators cop
We prefer space around operators, but `Layout/SpaceAroundOperators` cop
was temporarily disabled in #36943 since that cop changed to check
alignment strictly somehow.

In RuboCop 1.0.0, that is fixed by https://github.com/rubocop-hq/rubocop/pull/8906.

Related https://github.com/rails/rails/pull/38034#discussion_r359845661,
https://github.com/rails/rails/pull/39770#discussion_r448829561.
2020-10-23 16:12:15 +09:00
Alejandro Rodríguez Salamanca
fae2df5f54 Update database_configurations.rb 2020-10-13 09:43:12 +01:00
Guo Xiang Tan
04443b2408
Prioritize db_config for current env when resolving config. 2020-07-07 17:30:34 +08:00
eileencodes
79ce7d9af6
Deprecate spec_name and use name for configurations
I have so. many. regrets. about using `spec_name` for database
configurations and now I'm finally putting this mistake to an end.

Back when I started multi-db work I assumed that eventually
`connection_specification_name` (sometimes called `spec_name`) and
`spec_name` for configurations would one day be the same thing. After
2 years I no longer believe they will ever be the same thing.

This PR deprecates `spec_name` on database configurations in favor of
`name`. It's the same behavior, just a better name, or at least a
less confusing name.

`connection_specification_name` refers to the parent class name (ie
ActiveRecord::Base, AnimalsBase, etc) that holds the connection for it's
models. In some places like ConnectionHandler it shortens this to
`spec_name`, hence the major confusion.

Recently I've been working with some new folks on database stuff and
connection management and realize how confusing it was to explain that
`db_config.spec_name` was not `spec_name` and
`connection_specification_name`. Worse than that one is a symbole while
the other is a class name. This was made even more complicated by the
fact that `ActiveRecord::Base` used `primary` as the
`connection_specification_name` until #38190.

After spending 2 years with connection management I don't believe that
we can ever use the symbols from the database configs as a way to
connect the database without the class name being _somewhere_ because
a db_config does not know who it's owner class is until it's been
connected and a model has no idea what db_config belongs to it until
it's connected. The model is the only way to tie a primary/writer config
to a replica/reader config. This could change in the future but I don't
see value in adding a class name to the db_configs before connection or
telling a model what config belongs to it before connection. That would
probably break a lot of application assumptions. If we do ever end up in
that world, we can use name, because tbh `spec_name` and
`connection_specification_name` were always confusing to me.
2020-02-24 13:27:07 -05:00
eileencodes
2a53fe638d
Deprecate and replace #default_hash and #[]
Database configurations are now objects almost everywhere, so we don't
need to fake access to a hash with `#default_hash` or it's alias `#[]`.
Applications should `configs_for` and pass `env_name` and `spec_name` to
get the database config object. If you're looking for the default for
the test environment you can pass `configs_for(env_name: "test", spec_name:
"primary")`. Change test to developement to get the dev config, etc.

`#default_hash` and `#[]` will be removed in 6.2.

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
2020-01-17 16:08:12 -05:00
eileencodes
2791b7c280 Fix bug in configs_for
If a spec name was provided without an env name, config_for would return
the first config that matched the spec, regardless of environment name.

Now configs_for will return the database config that matches the
default env and requested spec name.

Additionally this commit has moved the default env call into a method
because I'm tired of typing so many lines every single time.

We considered either returning all configs that match that spec name or
raising an error if only spec was passed, but this change has the least
impact on current behavior and matches Active Record's assumptions: that
if you ask for configs it will always consider the current environment.

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
2019-12-03 11:38:12 -08:00
eileencodes
4e01932d43 Deprecate to_h and to_legacy_hash
These should have been deprecated when I added them but for some reason
I didn't.

As we move away from passing hashes around we no longer need these
methods, and since we no longer use configuration hashes as database
configuration we can deprecate this in favor of using the database
configuration objects and access the connection hashes from there.

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
2019-09-17 20:49:38 -04:00
eileencodes
ce9b197cc9 Use symbols everywhere for database configurations
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>
2019-09-13 08:53:22 -04:00
yuuji.yaginuma
121551b80a Avoid to use a method that acts on the hash
To avoid a deprecation warning.
2019-07-27 17:26:42 +09:00
Rafael França
344bed41d0
Merge pull request #36372 from instructure-bridge/6-0-stable
Don't break configurations.each, .first before the deprecation period
2019-07-26 12:59:27 -04:00
eileencodes
06f9434342 Improve errors and handling of hashes for database configurations
In chat Sam Saffron asked how to use the setter now that configurations
is no longer a hash and you need to do AR::Base.configurations["test"]=.

Technically you can do `ActiveRecord::Base.configurations = { the hash
}` but I realized the old way throws an error and is unintuitive.

To aid in the transition from hashes to objects this PR makes a few
changes:

1) Re-adds a deprecated hash setter `[]=` that will add a new hash
to the configurations list OR replace an existing hash if that
environment is already present. This won't be supported in future Rails
versions but a good error is important.

2) Changed to throw deprecation warnings on the methods we decided to support
for hash conversion and raise on the methods we don't support.

3) Refactored the setter/getter hash deprecation warnings messages and
rewrote them.

Getters message:

```
DEPRECATION WARNING: `ActiveRecord::Base.configurations` no longer
returns a hash. Methods that act on the hash like `values` are
deprecated and will be removed in Rails 6.1. Use the `configs_for`
method to collect and iterate over the database configurations.
```

Setter message:

```
DEPRECATION WARNING: Setting `ActiveRecord::Base.configurations` with
`[]=` is deprecated. Use `ActiveRecord::Base.configurations=` directly
to set the configurations instead.
```

4) Rewrote the legacy configurations test file to test all the public
methods in the DatabaseConfigurations class.
2019-02-14 08:25:52 -05:00