Merge pull request #7 from limebike/nick/negative_one_failover

Nick/negative one failover

.travis.yml

@@ -9,8 +9,8 @@ services:
   - postgresql
 script: bundle exec rake test
 before_script:
-  - psql -c 'create database distribute_reads_test_primary;' -U postgres
-  - psql -c 'create database distribute_reads_test_replica;' -U postgres
+  - createdb distribute_reads_test_primary
+  - createdb distribute_reads_test_replica
 notifications:
   email:
     on_success: never

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 0.3.0 [unreleased]
 
 - Use logger instead of stderr
+- Dropped support for Active Record < 5
 
 ## 0.2.4
 

distribute_reads.gemspec

@@ -13,14 +13,15 @@ Gem::Specification.new do |spec|
   spec.files         = Dir["*.{md,txt}", "{lib}/**/*"]
   spec.require_path  = "lib"
 
-  spec.required_ruby_version = ">= 2.2"
+  spec.required_ruby_version = ">= 2.4"
 
   spec.add_dependency "makara"
+  spec.add_dependency "activerecord", ">= 5"
 
   spec.add_development_dependency "bundler"
   spec.add_development_dependency "rake"
   spec.add_development_dependency "minitest"
-  spec.add_development_dependency "pg", "< 1"
+  spec.add_development_dependency "pg"
   spec.add_development_dependency "mysql2"
   spec.add_development_dependency "activejob"
 end

lib/distribute_reads.rb

@@ -47,7 +47,8 @@ def self.lag(connection: nil)
       log "Multiple replicas available, lag only reported for one"
     end
 
-    if %w(PostgreSQL PostGIS).include?(connection.adapter_name)
+    case connection.adapter_name
+    when "PostgreSQL", "PostGIS"
       # cache the version number
       @server_version_num ||= {}
       cache_key = connection.pool.object_id
@@ -66,7 +67,7 @@ def self.lag(connection: nil)
           ELSE EXTRACT (EPOCH FROM NOW() - pg_last_xact_replay_timestamp())
         END AS lag".squish
       ).first["lag"].to_f
-    elsif %w(MySQL Mysql2 Mysql2Spatial Mysql2Rgeo).include?(connection.adapter_name)
+    when "MySQL", "Mysql2", "Mysql2Spatial", "Mysql2Rgeo"
       replica_value = Thread.current[:distribute_reads][:replica]
       begin
         # makara doesn't send SHOW queries to replica, so we must force it
@@ -97,6 +98,8 @@ def self.lag(connection: nil)
       ensure
         Thread.current[:distribute_reads][:replica] = replica_value
       end
+    when "SQLite"
+      0.0
     else
       raise DistributeReads::Error, "Option not supported with this adapter"
     end
@@ -125,8 +128,8 @@ def self.default_to_primary=(value)
   end
 end
 
-Makara::Proxy.send :prepend, DistributeReads::AppropriatePool
-Object.send :include, DistributeReads::GlobalMethods
+Makara::Proxy.prepend DistributeReads::AppropriatePool
+Object.include DistributeReads::GlobalMethods
 
 ActiveSupport.on_load(:active_job) do
   require "distribute_reads/job_methods"

lib/distribute_reads/global_methods.rb

@@ -21,7 +21,7 @@ def distribute_reads(**options)
         if max_lag && !options[:primary]
           Array(options[:lag_on] || [ActiveRecord::Base]).each do |base_model|
             current_lag = DistributeReads.lag(connection: base_model.connection)
-            if current_lag > max_lag || current_lag.nil?
+            if current_lag.nil? || current_lag > max_lag
               message = "Replica lag over #{max_lag} seconds#{options[:lag_on] ? " on #{base_model.name} connection" : ""}"
               if current_lag.nil?
                 message = "Replica lag is nil on #{base_model.name}"

test/distribute_reads_test.rb

@@ -78,13 +78,13 @@ def test_max_lag_under
 
   def test_lag_negative_one
     error = assert_raises DistributeReads::TooMuchLag do
-      with_lag(-1) do
+      with_lag(nil) do
         distribute_reads(max_lag: 1) do
           assert_replica
         end
       end
     end
-    assert_equal "Replica lag is -1 on ActiveRecord::Base", error.message
+    assert_equal "Replica lag is nil on ActiveRecord::Base", error.message
   end
 
   def test_max_lag_under_not_stubbed
@@ -102,7 +102,7 @@ def test_lag_failover
   end
 
   def test_lag_failover_negative_one
-    with_lag(-1) do
+    with_lag(nil) do
       distribute_reads(max_lag: 1, lag_failover: true) do
         assert_primary
       end

test/gemfiles/activerecord50.gemfile

@@ -0,0 +1,6 @@
+source "https://rubygems.org"
+
+gemspec path: "../.."
+
+gem "activerecord", "~> 5.0.0"
+gem "activejob", "~> 5.0.0"

test/gemfiles/activerecord51.gemfile

@@ -0,0 +1,6 @@
+source "https://rubygems.org"
+
+gemspec path: "../.."
+
+gem "activerecord", "~> 5.1.0"
+gem "activejob", "~> 5.1.0"

test/gemfiles/activerecord52.gemfile

@@ -1,6 +1,5 @@
 source "https://rubygems.org"
 
-# Specify your gem's dependencies in distribute_reads.gemspec
 gemspec path: "../.."
 
 gem "activerecord", "~> 5.2.0"

test/gemfiles/makara3.gemfile

@@ -1,6 +1,5 @@
 source "https://rubygems.org"
 
-# Specify your gem's dependencies in distribute_reads.gemspec
 gemspec path: "../.."
 
 gem "makara", "~> 0.3.0"