diff --git a/README.md b/README.md index 32ef415e..ffe18396 100644 --- a/README.md +++ b/README.md @@ -307,7 +307,6 @@ Available configuration options are: - `inline_execution_respects_schedule` (boolean) Opt-in to future behavior of inline execution respecting scheduled jobs. Defaults to `false`. - `logger` ([Rails Logger](https://api.rubyonrails.org/classes/ActiveSupport/Logger.html)) lets you set a custom logger for GoodJob. It should be an instance of a Rails `Logger` (Default: `Rails.logger`). - `preserve_job_records` (boolean) keeps job records in your database even after jobs are completed. (Default: `true`) -- `smaller_number_is_higher_priority` (boolean) allows you to specifiy that jobs should be run in ascending order of priority (smallest priority numbers first). This will be enabled by default in the next major version of GoodJob (v4.0), but jobs with the highest priority number are run first by default in all earlier versions of GoodJob. - `retry_on_unhandled_error` (boolean) causes jobs to be re-queued and retried if they raise an instance of `StandardError`. Be advised this may lead to jobs being repeated infinitely ([see below for more on retries](#retries)). Instances of `Exception`, like SIGINT, will *always* be retried, regardless of this attribute’s value. (Default: `false`) - `on_thread_error` (proc, lambda, or callable) will be called when there is an Exception. It can be useful for logging errors to bug tracking services, like Sentry or Airbrake. Example: @@ -499,7 +498,9 @@ As a second example, you may wish to show a link to a log aggregator next to eac ### Job priority -Higher priority numbers run first in all versions of GoodJob v3.x and below. GoodJob v4.x will change job `priority` to give smaller numbers higher priority (default: `0`), in accordance with Active Job's definition of priority (see #524). To opt-in to this behavior now, set `config.good_job.smaller_number_is_higher_priority = true` in your GoodJob initializer or `application.rb`. +Smaller `priority` values have higher priority and run first (default: `0`), in accordance with [Active Job's definition of priority](https://github.com/rails/rails/blob/e17faead4f2aff28da079d50f02ea5b015322d5b/activejob/lib/active_job/core.rb#L22). + +Prior to GoodJob v4, this was reversed: higher priority numbers ran first in all versions of GoodJob v3.x and below. When migrating from v3 to v4, new behavior can be opted into by setting `config.good_job.smaller_number_is_higher_priority = true` in your GoodJob initializer or `application.rb`. ### Labelled jobs diff --git a/app/models/good_job/job.rb b/app/models/good_job/job.rb index 68628bb6..7b89f1f0 100644 --- a/app/models/good_job/job.rb +++ b/app/models/good_job/job.rb @@ -99,13 +99,7 @@ class Job < BaseRecord # @!method priority_ordered # @!scope class # @return [ActiveRecord::Relation] - scope :priority_ordered, (lambda do - if GoodJob.configuration.smaller_number_is_higher_priority - order('priority ASC NULLS LAST') - else - order('priority DESC NULLS LAST') - end - end) + scope :priority_ordered, -> { order('priority ASC NULLS LAST') } # Order jobs by created_at, for first-in first-out # @!method creation_ordered diff --git a/lib/good_job/configuration.rb b/lib/good_job/configuration.rb index 2626dc65..d7671c94 100644 --- a/lib/good_job/configuration.rb +++ b/lib/good_job/configuration.rb @@ -35,8 +35,6 @@ class Configuration DEFAULT_DASHBOARD_LIVE_POLL_ENABLED = true # Default enqueue_after_transaction_commit DEFAULT_ENQUEUE_AFTER_TRANSACTION_COMMIT = false - # Default smaller_number_is_higher_priority - DEFAULT_SMALLER_NUMBER_IS_HIGHER_PRIORITY = true def self.validate_execution_mode(execution_mode) raise ArgumentError, "GoodJob execution mode must be one of #{EXECUTION_MODES.join(', ')}. It was '#{execution_mode}' which is not valid." unless execution_mode.in?(EXECUTION_MODES) @@ -347,12 +345,6 @@ def enable_listen_notify DEFAULT_ENABLE_LISTEN_NOTIFY end - def smaller_number_is_higher_priority - return rails_config[:smaller_number_is_higher_priority] unless rails_config[:smaller_number_is_higher_priority].nil? - - DEFAULT_SMALLER_NUMBER_IS_HIGHER_PRIORITY - end - def dashboard_default_locale rails_config[:dashboard_default_locale] || DEFAULT_DASHBOARD_DEFAULT_LOCALE end diff --git a/spec/app/models/good_job/job_spec.rb b/spec/app/models/good_job/job_spec.rb index 99f381e1..22b655de 100644 --- a/spec/app/models/good_job/job_spec.rb +++ b/spec/app/models/good_job/job_spec.rb @@ -566,20 +566,10 @@ def job_params let!(:small_priority_job) { described_class.create!(priority: -50) } let!(:large_priority_job) { described_class.create!(priority: 50) } - it 'smaller_number_is_higher_priority=true orders with smaller number being HIGHER priority' do + it 'orders with smaller number being HIGHER priority' do allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: true }) expect(described_class.priority_ordered.pluck(:priority)).to eq([-50, 50]) end - - it 'smaller_number_is_higher_priority=false orders with smaller number being LOWER priority' do - allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: false }) - expect(described_class.priority_ordered.pluck(:priority)).to eq([50, -50]) - end - - it 'smaller_number_is_higher_priority=nil orders with smaller number being HIGHER priority' do - allow(Rails.application.config).to receive(:good_job).and_return({}) - expect(described_class.priority_ordered.pluck(:priority)).to eq([-50, 50]) - end end describe '.next_scheduled_at' do diff --git a/spec/lib/good_job/configuration_spec.rb b/spec/lib/good_job/configuration_spec.rb index f0903bb9..3bb0e4ba 100644 --- a/spec/lib/good_job/configuration_spec.rb +++ b/spec/lib/good_job/configuration_spec.rb @@ -298,14 +298,6 @@ end end - describe '#smaller_number_is_higher_priority' do - it 'delegates to rails configuration' do - allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: true }) - configuration = described_class.new({}) - expect(configuration.smaller_number_is_higher_priority).to be true - end - end - describe '#dashboard_default_locale' do it 'delegates to rails configuration' do allow(Rails.application.config).to receive(:good_job).and_return({ dashboard_default_locale: :de })