-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix Rails 7.1 deprecation warning #4
Conversation
DEPRECATION WARNING: `connection_pool_list` currently only applies to connection pools in the current role (`writing`). In Rails 7.2, this method will apply to all known pools, regardless of role. To affect only those connections belonging to a specific role, pass the role name as an argument. To switch to the new behavior, pass `:all` as the role name. (called from block (2 levels) in <module:ActiveRecord> at /Users/jacopobeschi/.rbenv/versions/3.3.0-preview2/lib/ruby/gems/3.3.0+0/gems/yabeda-activerecord-0.1.0/lib/yabeda/active_record.rb:74)
b21c252
to
29df0a3
Compare
connection_pools = ::ActiveRecord::Base.connection_handler.connection_pool_list | ||
connection_pools = \ | ||
if Rails.version >= "7.1" | ||
::ActiveRecord::Base.connection_handler.connection_pool_list(:all) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about this code but I assume we want to retrieve all roles here, if instead we want to only use the current role we can achieve it via ::ActiveRecord::Base.connection_handler.connection_pool_list(ActiveRecord::Base.current_role)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that your assumption is correct (but not sure myself)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your pull request and sorry for the long wait. There is just one little fix needed.
Co-authored-by: Andrey Novikov <envek@envek.name>
Co-authored-by: Andrey Novikov <envek@envek.name>
See https://github.com/rails/rails/blob/7-1-stable/activerecord/lib/active_record/connection_adapters/abstract/connection_handler.rb#L294-L300