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.

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

Support ActiveRecord 7.2 #53

Merged
merged 5 commits into from
Oct 15, 2024
Merged

Support ActiveRecord 7.2 #53

merged 5 commits into from
Oct 15, 2024

Conversation

a-lavis
Copy link
Contributor

@a-lavis a-lavis commented Oct 14, 2024

  1. Add Ruby 3.3 to CI
  2. Set up CI / local dev to test multiple versions of ActiveRecord. Test 7.1 and 7.2 on CI.
  3. Configure RSpec to use :focus metadata for ease of testing.
  4. Update QueryCache tests for 7.2. We need to accomodate the changes in this commit: rails/rails@85742ce
    #query_cache used to return a Hash, but it now returns a custom class. Fortunately, both Hash and this custom class implement a #size method which works the same as doing keys.size.
  5. Add with_connection_proxy method. ActiveRecord::Base now has the with_connection method, which is like the old connection method on ActiveRecord::Base but it yields the connection to a block instead of returning it: rails/rails@22f41a1
    This new method is now used for transactions, so in order for transactions to work we need to implement an equivalent in Replica Pools.

We need to accomodate the changes in this commit:
rails/rails@85742ce
`#query_cache` used to return a `Hash`, but it now returns a custom
class. Fortunately, both `Hash` and this custom class implement a
`#size` method which works the same as doing `keys.size`.
`ActiveRecord::Base` now has the `with_connection` method, which is like
the old `connection` method on `ActiveRecord::Base` but it yields the
connection to a block instead of returning it:
rails/rails@22f41a1

This new method is now used for transactions, so in order for
transactions to work we need to implement an equivalent in Replica
Pools.
Copy link
Contributor

@amancevice amancevice left a comment

Choose a reason for hiding this comment

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

looks good, but one minor change + question

.github/workflows/test-release.yml Outdated Show resolved Hide resolved

# Just helping out the bundler resolver:
gem "rails", "> 6.0"
end
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make more sense to make different groups instead of using ENV?

eg

group :rails7_1 do
  # ...
end

group :rails7_2 do
  # ...
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The nice thing about the ENV variable is that we can set it to any version of ActiveRecord without having to update the Gemfile. But happy to switch it to this if you prefer!

Copy link
Contributor Author

@a-lavis a-lavis Oct 15, 2024

Choose a reason for hiding this comment

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

With this diff:

diff --git a/Gemfile b/Gemfile
index 1c249f6..0170d9d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,15 +2,21 @@ source "https://rubygems.org"
gemspec

group :development do
-  # This allows us to easily switch between different versions of ActiveRecord.
-  # To use this in local dev, you can do:
-  # ```
-  # rm Gemfile.lock
-  # ACTIVE_RECORD_VERSION="7.1" bundle install
-  # ```
-  active_record_version = ENV.fetch("ACTIVE_RECORD_VERSION", nil)
-  gem "activerecord", "~> #{active_record_version}.0" if active_record_version&.length&.positive?
-
  # Just helping out the bundler resolver:
  gem "rails", "> 6.0"
end
+
+# This allows us to easily switch between different versions of ActiveRecord.
+# To use this in local dev, you can do:
+# ```
+# rm Gemfile.lock
+# bundle config set --local with activerecord_7_1
+# bundle install
+# ```
+group :activerecord_7_1, optional: true do
+  gem "activerecord", "~> 7.1.0"
+end
+
+group :activerecord_7_2, optional: true do
+  gem "activerecord", "~> 7.2.0"
+end

after running:

rm Gemfile.lock
bundle install

I'm getting this error

[!] There was an error parsing `Gemfile`: You cannot specify the same gem twice with different version requirements.
You specified: activerecord (~> 7.1.0) and activerecord (~> 7.2.0). Bundler cannot continue.

 #  from /Users/aidanlavis/git/replica_pools/Gemfile:21
 #  -------------------------------------------
 #  group :activerecord_7_2, optional: true do
 >    gem "activerecord", "~> 7.2.0"
 #  end
 #  -------------------------------------------

Running:

bundle config set --local with activerecord_7_1

does not help

@a-lavis a-lavis merged commit aa619e7 into main Oct 15, 2024
18 checks passed
@a-lavis a-lavis deleted the activerecord-7.2 branch October 15, 2024 14:12
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.

2 participants