Use min max job runtime between class & global

CHANGELOG.md

@@ -1,7 +1,13 @@
 ### Main (unreleased)
 
+### Changes
+
+- [437](https://github.com/Shopify/job-iteration/pull/437) - Use minimum between per-class `job_iteration_max_job_runtime` and `JobIteration.max_job_runtime`, instead of enforcing only setting decreasing values.
+  Because it is possible to change the global or parent values after setting the value on a class, it is not possible to truly enforce the decreasing value constraint. Instead, we now use the minimum between the global value and per-class value. This is considered a non-breaking change, as it should not break any **existing** code, it only removes the constraint on new classes.
+
 ### Bug fixes
 
+- [437](https://github.com/Shopify/job-iteration/pull/437) - Defer reading `JobIteration.max_job_runtime` until runtime, instead of closing around the value at the time of job definition.
 - [431](https://github.com/Shopify/job-iteration/pull/431) - Use `#id_value` instead of `send(:id)`
 when generating position for cursor based on `:id` column (Rails 7.1 and above, where composite
 primary models are now supported). This ensures we grab the value of the id column, rather than a

lib/job-iteration.rb

@@ -29,14 +29,22 @@ def logger
   # This setting will make it to always interrupt a job after it's been iterating for 5 minutes.
   # Defaults to nil which means that jobs will not be interrupted except on termination signal.
   #
-  # This setting can be further reduced (but not increased) by using the inheritable per-class
-  # job_iteration_max_job_runtime setting.
+  # This setting can be overriden by using the inheritable per-class job_iteration_max_job_runtime setting. At runtime,
+  # the lower of the two will be used.
   # @example
   #
   #   class MyJob < ActiveJob::Base
   #     include JobIteration::Iteration
   #     self.job_iteration_max_job_runtime = 1.minute
   #     # ...
+  #
+  # Note that if a sub-class overrides its parent's setting, only the global and sub-class setting will be considered,
+  # not the parent's.
+  # @example
+  #
+  #   class ChildJob < MyJob
+  #     self.job_iteration_max_job_runtime = 3.minutes # MyJob's 1.minute will be discarded.
+  #     # ...
   attr_accessor :max_job_runtime
 
   # Configures a delay duration to wait before resuming an interrupted job.

lib/job-iteration/iteration.rb

@@ -47,30 +47,9 @@ def inspected_cursor
 
       class_attribute(
         :job_iteration_max_job_runtime,
-        instance_writer: false,
+        instance_accessor: false,
         instance_predicate: false,
-        default: JobIteration.max_job_runtime,
       )
-
-      singleton_class.prepend(PrependedClassMethods)
-    end
-
-    module PrependedClassMethods
-      def job_iteration_max_job_runtime=(new)
-        existing = job_iteration_max_job_runtime
-
-        if existing && (!new || new > existing)
-          existing_label = existing.inspect
-          new_label = new ? new.inspect : "#{new.inspect} (no limit)"
-          raise(
-            ArgumentError,
-            "job_iteration_max_job_runtime may only decrease; " \
-              "#{self} tried to increase it from #{existing_label} to #{new_label}",
-          )
-        end
-
-        super
-      end
     end
 
     module ClassMethods
@@ -291,13 +270,22 @@ def instrumentation_tags
     end
 
     def job_should_exit?
-      if job_iteration_max_job_runtime && start_time && (Time.now.utc - start_time) > job_iteration_max_job_runtime
-        return true
-      end
+      max_job_runtime = job_iteration_max_job_runtime
+      return true if max_job_runtime && start_time && (Time.now.utc - start_time) > max_job_runtime
 
       JobIteration.interruption_adapter.call || (defined?(super) && super)
     end
 
+    def job_iteration_max_job_runtime
+      global_max = JobIteration.max_job_runtime
+      class_max = self.class.job_iteration_max_job_runtime
+
+      return global_max unless class_max
+      return class_max unless global_max
+
+      [global_max, class_max].min
+    end
+
     def handle_completed(completed)
       case completed
       when nil # someone aborted the job but wants to call the on_complete callback

test/unit/iteration_test.rb

@@ -311,6 +311,17 @@ def test_global_max_job_runtime
     end
   end
 
+  def test_global_max_job_runtime_with_updated_value
+    freeze_time
+    with_global_max_job_runtime(10.minutes) do
+      klass = build_slow_job_class(iterations: 3, iteration_duration: 30.seconds)
+      with_global_max_job_runtime(1.minute) do
+        klass.perform_now
+        assert_partially_completed_job(cursor_position: 2)
+      end
+    end
+  end
+
   def test_per_class_max_job_runtime_with_default_global
     freeze_time
     parent = build_slow_job_class(iterations: 3, iteration_duration: 30.seconds)
@@ -358,57 +369,56 @@ def test_per_class_max_job_runtime_with_global_set
     end
   end
 
-  def test_max_job_runtime_cannot_unset_global
+  def test_per_class_max_job_runtime_with_global_set_lower
+    freeze_time
     with_global_max_job_runtime(30.seconds) do
-      klass = Class.new(ActiveJob::Base) do
-        include JobIteration::Iteration
+      parent = build_slow_job_class(iterations: 3, iteration_duration: 30.seconds)
+      child = Class.new(parent) do
+        self.job_iteration_max_job_runtime = 1.minute
       end
 
-      error = assert_raises(ArgumentError) do
-        klass.job_iteration_max_job_runtime = nil
-      end
+      parent.perform_now
+      assert_partially_completed_job(cursor_position: 1)
+      ActiveJob::Base.queue_adapter.enqueued_jobs = []
 
-      assert_equal(
-        "job_iteration_max_job_runtime may only decrease; " \
-          "#{klass} tried to increase it from 30 seconds to nil (no limit)",
-        error.message,
-      )
+      child.perform_now
+      assert_partially_completed_job(cursor_position: 1)
     end
   end
 
-  def test_max_job_runtime_cannot_be_higher_than_global
-    with_global_max_job_runtime(30.seconds) do
-      klass = Class.new(ActiveJob::Base) do
-        include JobIteration::Iteration
+  def test_unset_per_class_max_job_runtime_and_global_set
+    freeze_time
+    with_global_max_job_runtime(1.minute) do
+      parent = build_slow_job_class(iterations: 3, iteration_duration: 30.seconds)
+      parent.job_iteration_max_job_runtime = 30.seconds
+      child = Class.new(parent) do
+        self.job_iteration_max_job_runtime = nil
       end
 
-      error = assert_raises(ArgumentError) do
-        klass.job_iteration_max_job_runtime = 1.minute
-      end
+      parent.perform_now
+      assert_partially_completed_job(cursor_position: 1)
+      ActiveJob::Base.queue_adapter.enqueued_jobs = []
 
-      assert_equal(
-        "job_iteration_max_job_runtime may only decrease; #{klass} tried to increase it from 30 seconds to 1 minute",
-        error.message,
-      )
+      child.perform_now
+      assert_partially_completed_job(cursor_position: 2)
     end
   end
 
-  def test_max_job_runtime_cannot_be_higher_than_parent
-    with_global_max_job_runtime(1.minute) do
-      parent = Class.new(ActiveJob::Base) do
-        include JobIteration::Iteration
-        self.job_iteration_max_job_runtime = 30.seconds
+  def test_unset_per_class_max_job_runtime_and_unset_global_and_set_parent
+    freeze_time
+    with_global_max_job_runtime(nil) do
+      parent = build_slow_job_class(iterations: 3, iteration_duration: 30.seconds)
+      parent.job_iteration_max_job_runtime = 30.seconds
+      child = Class.new(parent) do
+        self.job_iteration_max_job_runtime = nil
       end
-      child = Class.new(parent)
 
-      error = assert_raises(ArgumentError) do
-        child.job_iteration_max_job_runtime = 45.seconds
-      end
+      parent.perform_now
+      assert_partially_completed_job(cursor_position: 1)
+      ActiveJob::Base.queue_adapter.enqueued_jobs = []
 
-      assert_equal(
-        "job_iteration_max_job_runtime may only decrease; #{child} tried to increase it from 30 seconds to 45 seconds",
-        error.message,
-      )
+      child.perform_now
+      assert_empty(ActiveJob::Base.queue_adapter.enqueued_jobs)
     end
   end