Remove owner_name #38672
Merged
Remove owner_name #38672
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kamipo
reviewed
Mar 6, 2020
| handler = lookup_connection_handler(role) | ||
|
|
||
| handler.establish_connection(db_config) | ||
| handler.establish_connection(db_config, :default, owner_name) |
There was a problem hiding this comment.
Should :default be ActiveRecord::Base.default_pool_key?
There was a problem hiding this comment.
Good catch, I fixed this and the other one.
We don't actually need this since the only reason it exists is to pass the owning class name down to the `handler`. This removes a level of indirection and an unnecessary accessor on db_config. db_config shouldn't have to know what class owns it, so we can just remove this and pass it to the handler. The Symbol case is needed to preserve current behavior. This doesn't need a changelog because it's changing un-released behavior. Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
eileencodes
force-pushed
the
remove-owner_name
branch
from
a4de12c
to
7400d19
Compare
eileencodes
added a commit
to eileencodes/rails
that referenced
this pull request
Jul 5, 2022
The majority of where this functionality was removed in rails#38672. `owner_name` was never used publicaly in the db_config and was at a time used for identifying connection owners. Now the connection owner is always stored in the PoolConfig (same place as the db_config) so the db_config doesn't need this information.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We don't actually need this since the only reason it exists is to pass
the owning class name down to the
handler. This removes a level ofindirection and an unnecessary accessor on db_config. db_config
shouldn't have to know what class owns it, so we can just remove this
and pass it to the handler.
The Symbol case is needed to preserve current behavior. This doesn't
need a changelog because it's changing un-released behavior.
Co-authored-by: John Crepezzi john.crepezzi@gmail.com