Closed hasghari closed 2 years ago
I don't think it should matter when in the cli you use the -C
option. In general here's how the config works:
config/puma.rb
take precedent (or another config file you specify)That's intentional because it would be really confusing if someone was running:
puma --port 3000
And it booted on a different port.
If you want the values in the config to take precedent, don't provide them via to the puma
command.
We could make this more clear perhaps. For example a warning:
Warning: You are specifying `threads` count via `config/puma.rb` and via the flag `-t`
Though this is the first time i've seen this come up. It's not something I would add at this point, but might take a PR if it wasn't too massive.
Thanks @schneems. I think the challenge here is that puma-dev
controls the invocation of puma
and always provides all of those flags.
Upon reflection, I think the best course of action is to conditionally add -w
iff $WORKERS
is defined, -t
iff $THREADS
is defined and -C
iff $CONFIG
is defined. Something like:
WORKERS_ARG=$([ -z "$WORKERS" ] || echo "-w $WORKERS")
THREADS_ARG=$([ -z "$THREADS" ] || echo "-t 0:$THREADS")
CONFIG_ARG=$([ -z "$CONFIG" ] || echo "-C $CONFIG")
puma $WORKERS_ARG $THREADS_ARG $CONFIG_ARG
That means leaning into the puma defaults rather than trying to explicitly set them ourselves. https://github.com/puma/puma-dev/blob/fca1942e049c221fa6b7d850a64c22d53b5c4a85/dev/app.go#L287-L291
Then if you provide all 3 (via .env
or similar), you're doing it wrong. @hasghari does that approach seem reasonable to you?
@nonrational I agree that ideally puma-dev should only supply the workers and threads flags if they have been explicitly defined via environment variables.
If @schneems agrees with the suggested solution by @nonrational, I can update my pull request with that approach.
I agree that ideally puma-dev should only supply the workers and threads flags if they have been explicitly defined via environment variables.
Only took 18 months, but I've implemented this approach in https://github.com/puma/puma-dev/pull/302.
@nonrational Let's reopen this until we've got a fix merged.
I've opened https://github.com/puma/puma-dev/issues/307 to track this issue.
Currently when using the following puma config file, the
threads
andworkers
options are ignored because the-w
and-t
flags appear after the-C
flag and will override the workers and threads settings.The change in this PR allows the puma config file to be authoritative.