-
Notifications
You must be signed in to change notification settings - Fork 146
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 connection handling #194
Fix Rails 7 connection handling #194
Conversation
The monkey patched `connected_to` was introduced in Rails 6, so this monkey patch isn't needed for Rails 5.
Any plans for merging this PR? Thanks. |
This has fixed issues I was having in the Rails 7 upgrade. Thanks @calmyournerves! Do you have any bandwidth to fix the rubocop failures? If not I could take a look. |
Just pushed a commit that simplifies the specs a little bit and should fix the rubocop stuff ✌️ |
Thank you for reporting and helping out on this. I think that for the sake of compatibility we may need to have 2 versions of the monkeypatch one that takes in the database keyword arg (before rails 6.2) and the other that doesn't for after 6.2. |
@rpbaltazar Different monkey patches is a good idea 👍 |
Any plans for releasing this? |
+1 on the release |
Hey @rpbaltazar any chance of getting a release for this please? I'm happy to help out if you want to add me as a rubygems contributor? (https://rubygems.org/profiles/markedmondson) |
@markedmondson In case it helps, I work on a project where, as a workaround, we've added this code to the apartment initializer: module ActiveRecord
module ConnectionHandling
def connected_to_with_rails7_tenant(role: nil, prevent_writes: false, &blk)
current_tenant = Apartment::Tenant.current
# The connected_to_without_tenant method is defined by Apartment
connected_to_without_tenant(role: role, prevent_writes: prevent_writes) do
Apartment::Tenant.switch!(current_tenant)
yield(blk)
end
end
alias connected_to connected_to_with_rails7_tenant
end
end |
@markedmondson @javierm could you please try the latest version we just released? This should account for both Rails 7 and < 6.2 connection handling https://rubygems.org/gems/ros-apartment |
Related: #193
After trying to upgrade one of our apps to Rails 7 we saw some specs failing related to the monkey-patched
connected_to
method in lib/apartment/active_record/connection_handling.rb. Rails 7 removed thedatabase:
keyword forconnected_to
(rails/rails@ce629d9). This means apartment is currently not compatible with Rails 7.This PR removes the deprecated
database:
keyword from the monkey patch. I'm not sure if this has any implications for Rails 6 users.I also changed the monkey patch to be only included for ActiveRecord >= 6 (
connected_to
was introduced in Rails 6). I also attempted to write some specs for the monkey-patch.