Skip to content

Ensure puma config load process is respected #2794

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshuay03
Copy link

@joshuay03 joshuay03 commented Feb 11, 2025

Ref: puma/puma#3616

In v7, puma will be validating that its config objects are both loaded (loads config files) and clamped (resolves config procs and finalizes defaults) before its options are accessed to begin the launch process. This will be a breaking change for users utilising capybara as it builds its own puma config, but only clamps it. This change ensures a future version of capybara can be compatible with puma v7, while also being backward compatible with previous versions.

Note: The side effect of this change is that options specified in users' puma config files that are not specified in capybara's user options will be taken into account when capybara boots its server. A workaround would be to only pass in the user options hash here (conf.options.user_options) so only those options will be picked up by puma and merged with its default options. Let me know what you think.

@joshuay03 joshuay03 force-pushed the ensure-puma-config-is-loaded branch 2 times, most recently from b80100c to 055a470 Compare February 16, 2025 00:30
@joshuay03 joshuay03 changed the title Ensure puma config files are loaded Ensure puma config load process is respected Feb 16, 2025
@joshuay03 joshuay03 force-pushed the ensure-puma-config-is-loaded branch 2 times, most recently from 4e37e47 to 7292656 Compare February 16, 2025 00:55
@joshuay03
Copy link
Author

A workaround would be to only pass in the user options hash here (conf.options.user_options) so only those options will be picked up by puma and merged with its default options. Let me know what you think.

Actually, a much simpler workaround would be to specify config_files: ['-'] with the user options which will take precedence over the default file lookup. I've gone ahead and actioned this.

@joshuay03 joshuay03 force-pushed the ensure-puma-config-is-loaded branch from 7292656 to e5050a4 Compare February 16, 2025 00:59
@joshuay03 joshuay03 force-pushed the ensure-puma-config-is-loaded branch from e5050a4 to 2f692af Compare February 16, 2025 02:13
expect(Puma::Server).to have_received(:new).with(
anything,
anything,
satisfy { |opts| opts.final_options[:debug] == false }
Copy link
Author

@joshuay03 joshuay03 Feb 16, 2025

Choose a reason for hiding this comment

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

With load but without config_files override:

Screenshot 2025-02-16 at 12 13 06 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant