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
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
8 changes: 6 additions & 2 deletions .github/workflows/test-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ jobs:
- "2.7"
- "3.1"
- "3.2"
- "3.3"
activerecord:
- "7.1"
- "7.2"
steps:
- uses: actions/checkout@v3
a-lavis marked this conversation as resolved.
Show resolved Hide resolved
- uses: ruby/setup-ruby@v1
Expand All @@ -42,7 +46,7 @@ jobs:
- uses: actions/checkout@v3
- uses: ruby/setup-ruby@v1
with:
ruby-version: "3.2"
ruby-version: "3.3"
# The kickstarter/actions/setup-rubygems action is not available
# because this is a public repo
- name: setup-rubygems
Expand All @@ -53,5 +57,5 @@ jobs:
:rubygems_api_key: ${{ secrets.RUBYGEMS_API_KEY }}
YAML
chmod 0600 ~/.gem/credentials
- run: bundle install
- run: ACTIVE_RECORD_VERSION=${{ matrix.activerecord }} bundle install
- run: bundle exec rake release
14 changes: 14 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,2 +1,16 @@
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
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

2 changes: 1 addition & 1 deletion Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ dotenv:
- .env

vars:
RUBY_VERSION: 3.2.2
RUBY_VERSION: 3.3.5

tasks:
init:
Expand Down
4 changes: 4 additions & 0 deletions lib/replica_pools/active_record_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ def connection_proxy
ReplicaPools.proxy
end

def with_connection_proxy
yield ReplicaPools.proxy
end

# Make sure transactions run on leader
# Even if they're initiated from ActiveRecord::Base
# (which doesn't have our hijack).
Expand Down
1 change: 1 addition & 0 deletions lib/replica_pools/hijack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def inherited(child)
def hijack_connection
class << self
alias_method :connection, :connection_proxy
alias_method :with_connection, :with_connection_proxy
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions spec/query_cache_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@default_replica2.should_not_receive(:select_all)
@leader.should_not_receive(:select_all)
3.times { @proxy.select_all(@sql) }
@leader.query_cache.keys.size.should == 1
@leader.query_cache.size.should == 1
end
end

Expand All @@ -38,9 +38,9 @@
5.times do |i|
@proxy.select_all(@sql)
@proxy.select_all(@sql)
@leader.query_cache.keys.size.should == 1
@leader.query_cache.size.should == 1
@proxy.send(meths[i], '')
@leader.query_cache.keys.size.should == 0
@leader.query_cache.size.should == 0
end
end
end
Expand Down
3 changes: 3 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,7 @@ def reset_proxy(proxy)
c.mock_with :rspec do |c|
c.syntax = [:expect, :should]
end

c.filter_run focus: true
c.run_all_when_everything_filtered = true
end
Loading