From c7ab74d4956c81b344066fc57c4593d3badd2c41 Mon Sep 17 00:00:00 2001 From: "Ben Sheldon [he/him]" Date: Mon, 25 Jul 2022 08:50:29 -0700 Subject: [PATCH] Create global GoodJob.configuration object --- lib/good_job.rb | 11 ++++++++--- lib/good_job/adapter.rb | 24 +++++++++++++----------- lib/good_job/cli.rb | 8 ++++---- lib/good_job/configuration.rb | 7 ++++++- lib/models/good_job/cron_entry.rb | 2 +- spec/app/jobs/example_job_spec.rb | 2 +- spec/lib/good_job/adapter_spec.rb | 6 ++++++ spec/lib/good_job/cli_spec.rb | 8 +++----- spec/lib/good_job_spec.rb | 3 +-- 9 files changed, 43 insertions(+), 28 deletions(-) diff --git a/lib/good_job.rb b/lib/good_job.rb index f39c9ab83..b87b3ef74 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -69,6 +69,12 @@ module GoodJob # @return [Proc, nil] mattr_accessor :on_thread_error, default: nil + # @!attribute [rw] configuration + # @!scope class + # Global configuration object for GoodJob. + # @return [GoodJob::Configuration, nil] + mattr_accessor :configuration, default: GoodJob::Configuration.new({}) + # Called with exception when a GoodJob thread raises an exception # @param exception [Exception] Exception that was raised # @return [void] @@ -135,10 +141,9 @@ def self._shutdown_all(executables, method_name = :shutdown, timeout: -1) # @params older_than [nil,Numeric,ActiveSupport::Duration] Jobs older than this will be destroyed (default: +86400+). # @return [Integer] Number of jobs that were destroyed. def self.cleanup_preserved_jobs(older_than: nil) - configuration = GoodJob::Configuration.new({}) - older_than ||= configuration.cleanup_preserved_jobs_before_seconds_ago + older_than ||= GoodJob.configuration.cleanup_preserved_jobs_before_seconds_ago timestamp = Time.current - older_than - include_discarded = configuration.cleanup_discarded_jobs? + include_discarded = GoodJob.configuration.cleanup_discarded_jobs? ActiveSupport::Notifications.instrument("cleanup_preserved_jobs.good_job", { older_than: older_than, timestamp: timestamp }) do |payload| old_jobs = GoodJob::Job.where('finished_at <= ?', timestamp) diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index 3b0e131b8..de4f3c531 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -10,6 +10,8 @@ class Adapter # @return [Array, nil] cattr_reader :instances, default: [], instance_reader: false + attr_reader :execution_mode + # @param execution_mode [Symbol, nil] specifies how and where jobs should be executed. You can also set this with the environment variable +GOOD_JOB_EXECUTION_MODE+. # # - +:inline+ executes jobs immediately in whatever process queued them (usually the web server process). This should only be used in test and development environments. @@ -25,8 +27,8 @@ class Adapter # - +production+ and all other environments: +:external+ # def initialize(execution_mode: nil) - @configuration = GoodJob::Configuration.new({ execution_mode: execution_mode }) - @configuration.validate! + @execution_mode = (execution_mode || GoodJob.configuration.execution_mode).to_sym + GoodJob::Configuration.validate_execution_mode(@execution_mode) self.class.instances << self start_async if GoodJob.async_ready? @@ -82,7 +84,7 @@ def enqueue_at(active_job, timestamp) # @return [void] def shutdown(timeout: :default) timeout = if timeout == :default - @configuration.shutdown_timeout + GoodJob.configuration.shutdown_timeout else timeout end @@ -95,21 +97,21 @@ def shutdown(timeout: :default) # Whether in +:async+ execution mode. # @return [Boolean] def execute_async? - @configuration.execution_mode == :async_all || - (@configuration.execution_mode.in?([:async, :async_server]) && in_server_process?) + execution_mode == :async_all || + (execution_mode.in?([:async, :async_server]) && in_server_process?) end # Whether in +:external+ execution mode. # @return [Boolean] def execute_externally? - @configuration.execution_mode == :external || - (@configuration.execution_mode.in?([:async, :async_server]) && !in_server_process?) + execution_mode == :external || + (execution_mode.in?([:async, :async_server]) && !in_server_process?) end # Whether in +:inline+ execution mode. # @return [Boolean] def execute_inline? - @configuration.execution_mode == :inline + execution_mode == :inline end # Start async executors @@ -118,12 +120,12 @@ def start_async return unless execute_async? @notifier = GoodJob::Notifier.new - @poller = GoodJob::Poller.new(poll_interval: @configuration.poll_interval) - @scheduler = GoodJob::Scheduler.from_configuration(@configuration, warm_cache_on_initialize: true) + @poller = GoodJob::Poller.new(poll_interval: GoodJob.configuration.poll_interval) + @scheduler = GoodJob::Scheduler.from_configuration(GoodJob.configuration, warm_cache_on_initialize: true) @notifier.recipients << [@scheduler, :create_thread] @poller.recipients << [@scheduler, :create_thread] - @cron_manager = GoodJob::CronManager.new(@configuration.cron_entries, start_on_initialize: true) if @configuration.enable_cron? + @cron_manager = GoodJob::CronManager.new(GoodJob.configuration.cron_entries, start_on_initialize: true) if GoodJob.configuration.enable_cron? @_async_started = true end diff --git a/lib/good_job/cli.rb b/lib/good_job/cli.rb index 99174d24b..0413ff66a 100644 --- a/lib/good_job/cli.rb +++ b/lib/good_job/cli.rb @@ -85,7 +85,8 @@ def exit_on_failure? def start set_up_application! - configuration = GoodJob::Configuration.new(options) + GoodJob.configuration.options.merge!(options.symbolize_keys) + configuration = GoodJob.configuration Daemon.new(pidfile: configuration.pidfile).daemonize if configuration.daemonize? @@ -142,10 +143,9 @@ def start def cleanup_preserved_jobs set_up_application! + GoodJob.configuration.options.merge!(options.symbolize_keys) - configuration = GoodJob::Configuration.new(options) - - GoodJob.cleanup_preserved_jobs(older_than: configuration.cleanup_preserved_jobs_before_seconds_ago) + GoodJob.cleanup_preserved_jobs(older_than: GoodJob.configuration.cleanup_preserved_jobs_before_seconds_ago) end no_commands do diff --git a/lib/good_job/configuration.rb b/lib/good_job/configuration.rb index f702e53cb..1c0a98175 100644 --- a/lib/good_job/configuration.rb +++ b/lib/good_job/configuration.rb @@ -27,7 +27,12 @@ class Configuration # Default to not running cron DEFAULT_ENABLE_CRON = false + 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) + end + # The options that were explicitly set when initializing +Configuration+. + # It is safe to modify this hash in place; be sure to symbolize keys. # @return [Hash] attr_reader :options @@ -47,7 +52,7 @@ def initialize(options, env: ENV) end def validate! - 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) + self.class.validate_execution_mode(execution_mode) end # Specifies how and where jobs should be executed. See {Adapter#initialize} diff --git a/lib/models/good_job/cron_entry.rb b/lib/models/good_job/cron_entry.rb index 69ee0e2eb..63ad1cf00 100644 --- a/lib/models/good_job/cron_entry.rb +++ b/lib/models/good_job/cron_entry.rb @@ -13,7 +13,7 @@ class CronEntry attr_reader :params def self.all(configuration: nil) - configuration ||= GoodJob::Configuration.new({}) + configuration ||= GoodJob.configuration configuration.cron_entries end diff --git a/spec/app/jobs/example_job_spec.rb b/spec/app/jobs/example_job_spec.rb index f04ef918c..e0dea7faf 100644 --- a/spec/app/jobs/example_job_spec.rb +++ b/spec/app/jobs/example_job_spec.rb @@ -65,7 +65,7 @@ describe "SLOW_TYPE" do it 'sleeps for period' do - expect_any_instance_of(Object).to receive(:sleep) + expect_any_instance_of(described_class).to receive(:sleep) active_job = described_class.perform_later(described_class::SLOW_TYPE) diff --git a/spec/lib/good_job/adapter_spec.rb b/spec/lib/good_job/adapter_spec.rb index 5ca41a9fc..6eb2c8252 100644 --- a/spec/lib/good_job/adapter_spec.rb +++ b/spec/lib/good_job/adapter_spec.rb @@ -7,6 +7,12 @@ let(:good_job) { instance_double(GoodJob::Execution, queue_name: 'default', scheduled_at: nil) } describe '#initialize' do + it 'uses the global configuration value' do + allow(GoodJob.configuration).to receive(:execution_mode).and_return(:external) + adapter = described_class.new + expect(adapter.execution_mode).to eq(:external) + end + it 'guards against improper execution modes' do expect do described_class.new(execution_mode: :blarg) diff --git a/spec/lib/good_job/cli_spec.rb b/spec/lib/good_job/cli_spec.rb index bafe0562e..1f19cb0b1 100644 --- a/spec/lib/good_job/cli_spec.rb +++ b/spec/lib/good_job/cli_spec.rb @@ -3,11 +3,10 @@ RSpec.describe GoodJob::CLI do let(:scheduler_mock) { instance_double GoodJob::Scheduler, shutdown?: false, shutdown: nil } - let(:env) { {} } - let(:args) { [] } before do stub_const 'GoodJob::CLI::RAILS_ENVIRONMENT_RB', File.expand_path("spec/test_app/config/environment.rb") + allow(GoodJob).to receive(:configuration).and_return(GoodJob::Configuration.new({})) allow(GoodJob::Scheduler).to receive(:new).and_return scheduler_mock end @@ -38,11 +37,10 @@ describe 'max threads' do it 'defaults to --max_threads, GOOD_JOB_MAX_THREADS, RAILS_MAX_THREADS, database connection pool size' do allow(Kernel).to receive(:loop) - - cli = described_class.new([], { max_threads: 4 }, {}) - stub_const 'ENV', ENV.to_hash.merge({ 'RAILS_MAX_THREADS' => 3, 'GOOD_JOB_MAX_THREADS' => 2 }) + allow(GoodJob.configuration).to receive(:env).and_return(ENV.to_h.merge({ 'RAILS_MAX_THREADS' => 3, 'GOOD_JOB_MAX_THREADS' => 2 })) allow(ActiveRecord::Base.connection_pool).to receive(:size).and_return(1) + cli = described_class.new([], { max_threads: 4 }, {}) cli.start expect(GoodJob::Scheduler).to have_received(:new).with( diff --git a/spec/lib/good_job_spec.rb b/spec/lib/good_job_spec.rb index 2606df08a..43a8e93a2 100644 --- a/spec/lib/good_job_spec.rb +++ b/spec/lib/good_job_spec.rb @@ -73,8 +73,7 @@ end it "respects the cleanup_discarded_jobs? configuration" do - stub_const 'ENV', ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_DISCARDED_JOBS' => 'false' }) - + allow(described_class.configuration).to receive(:env).and_return ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_DISCARDED_JOBS' => 'false' }) destroyed_jobs_count = described_class.cleanup_preserved_jobs expect(destroyed_jobs_count).to eq 1