Skip to content

Use lease_connection on Rails 7.2+ in ActiveRecord5Adapter#894

Open
benaitcheson wants to merge 1 commit into
CanCanCommunity:developfrom
benaitcheson:fix/lease-connection
Open

Use lease_connection on Rails 7.2+ in ActiveRecord5Adapter#894
benaitcheson wants to merge 1 commit into
CanCanCommunity:developfrom
benaitcheson:fix/lease-connection

Conversation

@benaitcheson

Copy link
Copy Markdown

Closes #893.

Why

Rails 7.2 soft-deprecated ActiveRecord::Base.connection in favour of lease_connection, which better reflects that the caller is acquiring a connection from the pool rather than holding a permanent one (see rails/rails#51230). connection still works on 7.2+, but apps that opt into stricter connection-pool semantics (ActiveRecord.permanent_connection_checkout = :deprecated or :disallowed) see warnings or errors from ActiveRecord5Adapter#visit_nodes.

What

Prefer @model_class.lease_connection when defined; fall back to @model_class.connection on Rails < 7.2. The conditional uses respond_to?(:lease_connection) rather than a Rails-version check so the same code path works against ActiveRecord main without further changes. This is the pattern used by other gems spanning the 7.2 boundary, e.g. good_job and pg_ha_migrations.

Backward compatibility

No behavioural change on Rails 5.2 / 6.x / 7.0 / 7.1 — lease_connection is undefined there and the fallback path keeps calling connection. The existing CI matrix should cover the fallback.

Rails 7.2 soft-deprecated ActiveRecord::Base.connection in favour of
lease_connection, which better describes that the caller is acquiring
a connection from the pool rather than holding a permanent one (see
rails/rails#51230). Prefer lease_connection when available, and fall
back to connection on Rails < 7.2 where it is not defined.

Refs CanCanCommunity#893

@duderamos duderamos left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean and clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use lease_connection on Rails 7.2+ in ActiveRecord5Adapter#visit_nodes

2 participants