Skip to content
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

Deprecate ConnectionPool#connection #51230

Merged
merged 1 commit into from Mar 1, 2024
Merged

Conversation

casperisfine
Copy link
Contributor

@casperisfine casperisfine commented Mar 1, 2024

Replaced by #lease_connection to better reflect what it does.

ActiveRecord::Base.connection is deprecated in the same way but without a removal timeline nor a deprecation warning.

Inside the Active Record test suite, we do remove Base.connection to ensure it's not used internally.

Some callsites have been converted to use with_connection, some other have been more simply migrated to lease_connection and will serve as a list of callsites to convert for #50793

FYI: @matthewd

Replaced by `#lease_connection` to better reflect what it does.

`ActiveRecord::Base#connection` is deprecated in the same way
but without a removal timeline nor a deprecation warning.

Inside the Active Record test suite, we do remove `Base.connection`
to ensure it's not used internally.

Some callsites have been converted to use `with_connection`,
some other have been more simply migrated to `lease_connection`
and will serve as a list of callsites to convert for
rails#50793
@byroot byroot merged commit e27a0df into rails:main Mar 1, 2024
3 of 4 checks passed
alias_method :connection, :lease_connection # TODO: deprecate
def connection
ActiveRecord.deprecator.warn(<<~MSG)
ConnectionPoool#connection is deprecated and will be removed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I wasn't even meaning this. I just noticed that Poool doesn't look very correct. My bad, should have been more specific.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I meant to fix that too, but I must be tired 😫

aidanharan added a commit to rails-sqlserver/activerecord-sqlserver-adapter that referenced this pull request Mar 11, 2024
aidanharan added a commit to rails-sqlserver/activerecord-sqlserver-adapter that referenced this pull request Mar 14, 2024
tagliala added a commit to ifad/chronomodel that referenced this pull request Mar 29, 2024
tagliala added a commit to ifad/chronomodel that referenced this pull request Mar 29, 2024
tagliala added a commit to ifad/chronomodel that referenced this pull request Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants