Skip to content

Commit

Permalink
Remove smaller_number_is_higher_priority option from v4 (#1453)
Browse files Browse the repository at this point in the history
It does not need to be deprecated because it is only set in the configuration hash.
  • Loading branch information
bensheldon authored Aug 1, 2024
1 parent 0e758d7 commit a2e6fab
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 36 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions lib/good_job/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
12 changes: 1 addition & 11 deletions spec/app/models/good_job/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions spec/lib/good_job/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down

0 comments on commit a2e6fab

Please sign in to comment.