Puma config get evaluated twice with pumactl

[madejejej]

Describe the bug

There's a difference in behavior in puma and pumactl for non-Puma specific code.

I'm not sure if I'd categorize it as a bug, since I can empathize with the thought it's not Puma's responsibility as the APIs are not Puma-specific. However, seeing our at_exit hook being called twice was surprising.

Puma config:

# puma.rb
app { [200, {}, ["OK"]]  }
$stderr.puts "starting"
at_exit { $stderr.puts "exiting" }

To Reproduce

Run either puma or pumactl and send a SIGINT signal:

puma -C puma.rb start > /dev/null
starting
^Cexiting

pumactl -F puma-repro.rb start > /dev/null
starting
starting
^Cexiting
exiting

Expected behavior
The config should get evaluated only once, both using puma and pumactl.

Desktop (please complete the following information):

• OS: Mac
• Puma Version 6.4.2

[lcowell]

The config file is evaluated in 2 places when called from bin/pumactl.

1. Puma::ControlCLI.new in the initializer
2. Puma::CLI.new specifically in a call to setup_options

In both cases, config files are evaluated by the Configuration class and will pluck the same options from the config files. So this isn't a case of the different bins needing to support different sets of options.

bin/pumactl will call ControlCLI#run will call CLI#run. If @config_file is set (it always is), it passes that value through to CLI#run.

This does appear to be a double evaluation of configuration, but what is the right thing to do here? We could pass the parsed configuration through the CLI instead of just passing @config_file. This would eliminate the double evaluation, but adds complexity to CLI.new as it now needs to handle a config file argument or processed config.

[dentarg]

Sounds like the complexity is needed to do the right thing? Or are we lacking one layer of abstraction?

[lcowell]

This does feel like necessary complexity. The least intrusive way I can think to handle this scenario is to add an additional, optional, positional argument to CLI.initialize to accept a Configuration object. If nothing is provided there's no behaviour change in CLI.

1. If a Configuration object is provided to CLI.new, we skip processing the config file again. We still need to step through the other configuration code, but provide these options as default/base values.
2. We could accept any object that responds to final_options, which is the only method we need to be able to call on Configuration.
3. If some other object type is provided in place of the default we should raise an error. Orrrr perhaps we can avoid the checking code as an exception will be raised if someone passes in the wrong thing anyways.

Did you have any abstraction ideas in mind?