Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix deprecated call in activerecord adapter #477

Closed

Conversation

nbarthel
Copy link

Deprecated call ::ActiveRecord::Base.clear_active_connections! has been moved to connection_handler.

This was causing our app to fail in production on Rails 7.2.1.

Not sure how well 7.2.1 is supported yet. Perhaps more PRs to come.

@@ -304,7 +304,7 @@ def destroy(model_instance)
end

def close
::ActiveRecord::Base.clear_active_connections!
::ActiveRecord::Base.connection_handler.clear_active_connections!
Copy link
Contributor

Choose a reason for hiding this comment

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

if ActiveRecord.version > '7.2'
::ActiveRecord::Base.connection_handler.clear_active_connections!
else
::ActiveRecord::Base.clear_active_connections!
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's assume that not everyone would be able to upgrade to 7.2+ as fast as we need to do it ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

as a quick fix, in the meantime, we could use something like the following:
ActiveRecord::Base.class.delegate :clear_active_connections!, to: :connection_handler
somewhere in an initializer, or such.

Copy link
Contributor

Choose a reason for hiding this comment

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

or a much cleaner version:

ActiveRecord::Base.define_singleton_method :clear_active_connections! do
  connection_handler.clear_active_connections!
end

@jkeen
Copy link
Collaborator

jkeen commented Sep 10, 2024

@nbarthel Mind making that change and updating the PR? I'll release a new version after that.

I like the clarity of the first version @mihaimuntenas suggested.

@mihaimuntenas
Copy link
Contributor

@jkeen I opened #478 to speed things up.

@jkeen
Copy link
Collaborator

jkeen commented Sep 11, 2024

Fixed in #478

@jkeen jkeen closed this Sep 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants