diff --git a/lib/solid_queue/configuration.rb b/lib/solid_queue/configuration.rb index b547d12c..d04a0aaa 100644 --- a/lib/solid_queue/configuration.rb +++ b/lib/solid_queue/configuration.rb @@ -3,9 +3,12 @@ module SolidQueue class Configuration include ActiveModel::Model + include ActiveModel::Validations::Callbacks - validate :ensure_configured_processes - validate :ensure_valid_recurring_tasks + validate :ensure_configured_processes, :ensure_valid_recurring_tasks + validate :warn_about_incorrectly_sized_thread_pool, :warn_about_missing_config_files + + before_validation { warnings.clear } class Process < Struct.new(:kind, :attributes) def instantiate @@ -46,43 +49,30 @@ def configured_processes end end - def error_messages - if configured_processes.none? - "No workers or processed configured. Exiting..." - else - error_messages = invalid_tasks.map do |task| - all_messages = task.errors.full_messages.map { |msg| "\t#{msg}" }.join("\n") - "#{task.key}:\n#{all_messages}" - end - .join("\n") - - "Invalid processes configured:\n#{error_messages}" - end - end - def mode - @options[:mode].to_s.inquiry + options[:mode].to_s.inquiry end def standalone? - mode.fork? || @options[:standalone] + mode.fork? || options[:standalone] end def warnings - @warnings ||= [ undersized_thread_pool_message ].compact + @warnings ||= ActiveModel::Errors.new(self) end def check - warnings.each { |warning| $stdout.puts "Warning: #{warning}" } - if valid? + warnings.full_messages.each { |warning| $stderr.puts warning } $stdout.puts "Solid Queue configuration is valid." + true else - $stderr.puts "Invalid Solid Queue configuration:" - errors.full_messages.each do |message| + $stderr.puts "Solid Queue configuration is invalid:" + (warnings.full_messages + errors.full_messages).each do |message| message.each_line { |line| $stderr.puts " #{line.chomp}" } end + false end end @@ -90,18 +80,6 @@ def check private attr_reader :options - def undersized_thread_pool_message - db_pool_size = SolidQueue::Record.connection_pool&.size - - if db_pool_size && db_pool_size < estimated_number_of_threads - "Solid Queue is configured to use #{estimated_number_of_threads} threads but the " \ - "database connection pool is #{db_pool_size}. Increase it in `config/database.yml`" - end - rescue ActiveRecord::ActiveRecordError - # No usable database connection. Skip the pool-size warning in that case. - nil - end - def ensure_configured_processes unless configured_processes.any? errors.add(:base, "No processes configured") @@ -118,6 +96,28 @@ def ensure_valid_recurring_tasks end end + def warn_about_incorrectly_sized_thread_pool + db_pool_size = SolidQueue::Record.connection_pool&.size + + if db_pool_size && db_pool_size < estimated_number_of_threads + warnings.add(:base, "Warning: Solid Queue is configured to use #{estimated_number_of_threads} threads but the " \ + "database connection pool is #{db_pool_size}. Increase it in `config/database.yml`") + end + rescue ActiveRecord::ActiveRecordError + # No usable database connection. Skip the pool-size warning in that case. + end + + def warn_about_missing_config_files + files = [ options[:config_file] ] + files << options[:recurring_schedule_file] unless skip_recurring_tasks? + + files.compact.each do |file| + unless Pathname.new(file).exist? + warnings.add(:base, "Warning: provided configuration file '#{file}' does not exist. Falling back to default configuration.") + end + end + end + def default_options { mode: ENV["SOLID_QUEUE_SUPERVISOR_MODE"] || :fork, @@ -244,7 +244,6 @@ def load_config_from_file(file) if file.exist? ActiveSupport::ConfigurationFile.parse(file).deep_symbolize_keys else - puts "[solid_queue] WARNING: Provided configuration file '#{file}' does not exist. Falling back to default configuration." {} end end diff --git a/lib/solid_queue/supervisor.rb b/lib/solid_queue/supervisor.rb index ffd4d3bd..17b90c54 100644 --- a/lib/solid_queue/supervisor.rb +++ b/lib/solid_queue/supervisor.rb @@ -13,7 +13,7 @@ def start(**options) configuration = Configuration.new(**options) if configuration.valid? - configuration.warnings.each { |warning| SolidQueue.logger.warn(warning) } + configuration.warnings.full_messages.each { |warning| SolidQueue.logger.warn(warning) } klass = configuration.mode.fork? ? ForkSupervisor : AsyncSupervisor klass.new(configuration).tap(&:start) diff --git a/lib/solid_queue/tasks.rb b/lib/solid_queue/tasks.rb index 026cb42e..70422562 100644 --- a/lib/solid_queue/tasks.rb +++ b/lib/solid_queue/tasks.rb @@ -11,6 +11,7 @@ desc "validate the Solid Queue configuration for the current Rails env without starting any process" task check: :environment do - exit 1 unless SolidQueue::Configuration.new.check + configuration = SolidQueue::Configuration.new + exit 1 unless configuration.check end end diff --git a/test/unit/cli_test.rb b/test/unit/cli_test.rb index 12e97dcf..f3d5415a 100644 --- a/test/unit/cli_test.rb +++ b/test/unit/cli_test.rb @@ -51,7 +51,7 @@ class CliTest < ActiveSupport::TestCase ) assert_equal 1, exit_status - assert_match "Invalid Solid Queue configuration", err + assert_match "Solid Queue configuration is invalid", err assert_match "periodic_invalid_class", err assert_match "periodic_incorrect_schedule", err assert_empty out diff --git a/test/unit/configuration_test.rb b/test/unit/configuration_test.rb index aeec5e5b..7bb7f70a 100644 --- a/test/unit/configuration_test.rb +++ b/test/unit/configuration_test.rb @@ -27,10 +27,11 @@ class ConfigurationTest < ActiveSupport::TestCase end test "warns if provided configuration file does not exist" do - assert_output "[solid_queue] WARNING: Provided configuration file '/path/to/nowhere.yml' does not exist. Falling back to default configuration.\n" do - configuration = SolidQueue::Configuration.new(config_file: Pathname.new("/path/to/nowhere.yml")) - assert configuration.valid? - end + configuration = SolidQueue::Configuration.new(config_file: Pathname.new("/path/to/nowhere.yml")) + + assert configuration.valid? + assert_includes configuration.warnings.full_messages, + "Warning: provided configuration file '/path/to/nowhere.yml' does not exist. Falling back to default configuration." end test "read configuration from default file" do @@ -145,20 +146,20 @@ class ConfigurationTest < ActiveSupport::TestCase assert error.include?("periodic_invalid_class: Class name doesn't correspond to an existing class") assert error.include?("periodic_incorrect_schedule: Schedule is not a supported recurring schedule") - assert_output(/Provided configuration file '[^']+' does not exist\./) do - assert SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:empty_recurring)).valid? - end + configuration = SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:empty_recurring)) + assert configuration.valid? + assert_match(/provided configuration file '[^']+' does not exist\./, configuration.warnings.full_messages.join) + assert SolidQueue::Configuration.new(skip_recurring: true).valid? configuration = SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:recurring_with_production_only)) assert configuration.valid? assert_processes configuration, :scheduler, 0 - assert_output(/Provided configuration file '[^']+' does not exist\./) do - configuration = SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:recurring_with_empty)) - assert configuration.valid? - assert_processes configuration, :scheduler, 0 - end + configuration = SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:recurring_with_empty)) + assert configuration.valid? + assert_processes configuration, :scheduler, 0 + assert_match(/provided configuration file '[^']+' does not exist\./, configuration.warnings.full_messages.join) # No processes configuration = SolidQueue::Configuration.new(skip_recurring: true, dispatchers: [], workers: []) @@ -175,15 +176,24 @@ class ConfigurationTest < ActiveSupport::TestCase assert configuration.valid? assert_equal 1, configuration.warnings.size - assert_match /Solid Queue is configured to use \d+ threads but the database connection pool is \d+\. Increase it in `config\/database.yml`/, configuration.warnings.first + assert_match /Solid Queue is configured to use \d+ threads but the database connection pool is \d+\. Increase it in `config\/database.yml`/, configuration.warnings.full_messages.first end test "has no warnings when the database connection pool is large enough" do configuration = SolidQueue::Configuration.new(workers: [ { queues: "background", threads: 1, polling_interval: 10 } ], skip_recurring: true) + assert configuration.valid? assert_empty configuration.warnings end + test "does not duplicate warnings when validated more than once" do + configuration = SolidQueue::Configuration.new(workers: [ { queues: "background", threads: 50, polling_interval: 10 } ], skip_recurring: true) + + 3.times { configuration.valid? } + + assert_equal 1, configuration.warnings.size + end + test "check prints a success message and returns true for a valid configuration" do out, err = capture_io do assert SolidQueue::Configuration.new(skip_recurring: true).check @@ -193,13 +203,13 @@ class ConfigurationTest < ActiveSupport::TestCase assert_empty err end - test "check prints warnings to stdout on the valid path" do - out, _err = capture_io do + test "check prints warnings to stderr on the valid path" do + out, err = capture_io do assert SolidQueue::Configuration.new(workers: [ { queues: "background", threads: 50, polling_interval: 10 } ], skip_recurring: true).check end assert_match "Solid Queue configuration is valid.", out - assert_match /Warning: Solid Queue is configured to use \d+ threads but the database connection pool is \d+/, out + assert_match /Solid Queue is configured to use \d+ threads but the database connection pool is \d+/, err end test "check prints errors to stderr and returns false for an invalid configuration" do @@ -207,7 +217,7 @@ class ConfigurationTest < ActiveSupport::TestCase assert_not SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:recurring_with_invalid)).check end - assert_match "Invalid Solid Queue configuration:", err + assert_match "Solid Queue configuration is invalid:", err assert_match "periodic_invalid_class", err end diff --git a/test/unit/rake_tasks_test.rb b/test/unit/rake_tasks_test.rb index 8b2de143..717bd980 100644 --- a/test/unit/rake_tasks_test.rb +++ b/test/unit/rake_tasks_test.rb @@ -44,7 +44,7 @@ class RakeTasksTest < ActiveSupport::TestCase assert_equal 1, status assert_empty out - assert_match "Invalid Solid Queue configuration:", err + assert_match "Solid Queue configuration is invalid:", err assert_match "broken", err end end