Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check Config File Existence #4054

Merged
merged 4 commits into from Dec 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 34 additions & 26 deletions lib/sidekiq/cli.rb
Expand Up @@ -27,7 +27,7 @@ class CLI
attr_accessor :launcher
attr_accessor :environment

def parse(args=ARGV)
def parse(args = ARGV)
setup_options(args)
initialize_logger
validate!
Expand Down Expand Up @@ -227,17 +227,33 @@ def symbolize_keys_deep!(hash)
alias_method :☠, :exit

def setup_options(args)
# parse CLI options
opts = parse_options(args)

set_environment opts[:environment]

options[:queues] << 'default' if options[:queues].empty?
# check config file presence
if opts[:config_file]
if opts[:config_file] && !File.exist?(opts[:config_file])
raise ArgumentError, "No such file #{opts[:config_file]}"
end
else
if File.directory?(opts[:require])
%w[config/sidekiq.yml config/sidekiq.yml.erb].each do |filename|
path = File.expand_path(filename, opts[:require])
opts[:config_file] ||= path if File.exist?(path)
end
end
end

cfile = opts[:config_file]
opts = parse_config(cfile).merge(opts) if cfile
# parse config file options
opts = parse_config(opts[:config_file]).merge(opts) if opts[:config_file]

opts[:queues] = Array(opts[:queues]) << 'default' if opts[:queues].nil? || opts[:queues].empty?
opts[:strict] = true if opts[:strict].nil?
opts[:concurrency] = Integer(ENV["RAILS_MAX_THREADS"]) if !opts[:concurrency] && ENV["RAILS_MAX_THREADS"]
opts[:concurrency] = Integer(ENV["RAILS_MAX_THREADS"]) if opts[:concurrency].nil? && ENV["RAILS_MAX_THREADS"]

# merge with defaults
options.merge!(opts)
end

Expand Down Expand Up @@ -367,11 +383,8 @@ def parse_options(argv)
logger.info @parser
die 1
end
@parser.parse!(argv)

%w[config/sidekiq.yml config/sidekiq.yml.erb].each do |filename|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing code that is potentially the issue.

opts[:config_file] ||= filename if File.exist?(filename)
end
@parser.parse!(argv)

opts
end
Expand All @@ -391,23 +404,18 @@ def write_pid
end
end

def parse_config(cfile)
opts = {}
if File.exist?(cfile)
opts = YAML.load(ERB.new(IO.read(cfile)).result) || opts
def parse_config(path)
opts = YAML.load(ERB.new(File.read(path)).result) || {}

if opts.respond_to? :deep_symbolize_keys!
opts.deep_symbolize_keys!
else
symbolize_keys_deep!(opts)
end

opts = opts.merge(opts.delete(environment.to_sym) || {})
parse_queues(opts, opts.delete(:queues) || [])
if opts.respond_to? :deep_symbolize_keys!
opts.deep_symbolize_keys!
else
# allow a non-existent config file so Sidekiq
# can be deployed by cap with just the defaults.
symbolize_keys_deep!(opts)
end

opts = opts.merge(opts.delete(environment.to_sym) || {})
parse_queues(opts, opts.delete(:queues) || [])

ns = opts.delete(:namespace)
if ns
# logger hasn't been initialized yet, puts is all we have.
Expand All @@ -421,10 +429,10 @@ def parse_queues(opts, queues_and_weights)
queues_and_weights.each { |queue_and_weight| parse_queue(opts, *queue_and_weight) }
end

def parse_queue(opts, q, weight=nil)
def parse_queue(opts, queue, weight = nil)
opts[:queues] ||= []
raise ArgumentError, "queues: #{q} cannot be defined twice" if opts[:queues].include?(q)
[weight.to_i, 1].max.times { opts[:queues] << q }
raise ArgumentError, "queues: #{queue} cannot be defined twice" if opts[:queues].include?(queue)
[weight.to_i, 1].max.times { opts[:queues] << queue }
opts[:strict] = false if weight.to_i > 0
end
end
Expand Down
2 changes: 2 additions & 0 deletions test/dummy/config/sidekiq.yml
@@ -0,0 +1,2 @@
---
:concurrency: 25
21 changes: 20 additions & 1 deletion test/test_cli.rb
Expand Up @@ -244,6 +244,17 @@ class TestCLI < Minitest::Test
assert_equal 3, Sidekiq.options[:queues].count { |q| q == 'seldom' }
end
end

describe 'default config file' do
describe 'when required path is a directory' do
it 'tries config/sidekiq.yml' do
@cli.parse(%w[sidekiq -r ./test/dummy])

assert_equal 'sidekiq.yml', File.basename(Sidekiq.options[:config_file])
assert_equal 25, Sidekiq.options[:concurrency]
end
end
end
end
end

Expand All @@ -260,6 +271,14 @@ class TestCLI < Minitest::Test
exit = assert_raises(SystemExit) { @cli.parse(%w[sidekiq -r ./test/fixtures]) }
assert_equal 1, exit.status
end

describe 'when config file path does not exist' do
it 'raises argument error' do
assert_raises(ArgumentError) do
@cli.parse(%w[sidekiq -r ./test/fake_env.rb -C /non/existent/path])
end
end
end
end
end
end
Expand All @@ -277,7 +296,7 @@ class TestCLI < Minitest::Test
after do
Sidekiq.logger = @logger
end

describe 'pidfile' do
it 'writes process pid to file' do
Sidekiq.options[:pidfile] = '/tmp/sidekiq.pid'
Expand Down