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.

Sign up for GitHub

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

Jump to bottom

Fix deprecation warning in ActiveRecord 7.1 when clearing active connections #498

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix deprecation warning in ActiveRecord 7.1 when clearing active connections #498

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e06e2f8
Fix deprecation warning in Active Record 7.1
haffla May 6, 2024
5d098ca
Merge branch 'master' into patch-1
haffla Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/scout_apm/app_server_load.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def data
ensure
# Sometimes :database_engine and :database_adapter can cause a reference to an AR connection.
# Make sure we release all AR connections held by this thread.
ActiveRecord::Base.connection_handler.clear_active_connections! if Utils::KlassHelper.defined?("ActiveRecord::Base")
ActiveRecord::Base.connection_handler.clear_active_connections!(:all) if Utils::KlassHelper.defined?("ActiveRecord::Base")
Copy link

Choose a reason for hiding this comment

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

Hello - I wanted to open the same PR (when running tests this deprecation warning gets quite verbose). In any case I think this is a safer way to implement this (backwards compatibility):

      if ActiveRecord::Base.connection_handler.respond_to?(:clear_active_connections!)
        ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
      elsif ActiveRecord::Base.respond_to?(:clear_active_connections!)
        # ~> Rails 3.2 -> https://apidock.com/rails/ActiveRecord/Base/clear_active_connections%21/class
        ActiveRecord::Base.clear_active_connections!
      end

Copy link
Author

@haffla haffla Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@marianposaceanu the link you posted: https://apidock.com/rails/ActiveRecord/Base/clear_active_connections%21/class actually shows that ActiveRecord::Base#clear_active_connections! just calls connection_handler.clear_active_connections!. So isn't it really the same?

Also there is a test for Rails 3 here and it's green.

Copy link
Author

Choose a reason for hiding this comment

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

Ah you mean that clear_active_connections! doesn't take any arguments in Rails 3? That could be actually. But I wonder why no test is failing. I am gonna look into it.

Copy link
Author

Choose a reason for hiding this comment

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

Oh lol, this code isn't even tested?!

Copy link
Author

Choose a reason for hiding this comment

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

OK so I think before touching this part, we would have to write some unit tests.

end

# Calls `.to_s` on the object passed in.
Expand Down