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

Warn when clustered-only hooks are defined in single mode #3089

Merged

Conversation

Vuta
Copy link
Contributor

@Vuta Vuta commented Feb 28, 2023

Description

Address #2950

  • Add a default warning log when clustered-only hooks are defined but puma is booted in single mode.
  • Add an config option to opt-out the default warning

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

lib/puma/dsl.rb Show resolved Hide resolved
test/test_config.rb Show resolved Hide resolved
@Vuta Vuta force-pushed the warn_when_hooks_defined_in_single_mode branch from a3cbbd9 to b0ac31e Compare February 28, 2023 17:25
Copy link
Member

@nateberkopec nateberkopec left a comment

Choose a reason for hiding this comment

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

One small change in the log string. I would also like to see some tests that the logstring is actually logged for each of the hooks you've added it to.

Thank you so much for contributing to Puma! An excellent first contribution.

lib/puma/dsl.rb Outdated Show resolved Hide resolved
@Vuta Vuta force-pushed the warn_when_hooks_defined_in_single_mode branch from b0ac31e to 5d8afd4 Compare March 1, 2023 12:30
…le mode

Clustered-only hooks won't be called in single mode, so this commit adds a log to warn users about that.
Also adds `silence_fork_callback_warning` option to opt-out of the default warning log.
@Vuta Vuta force-pushed the warn_when_hooks_defined_in_single_mode branch from 5d8afd4 to 4340f72 Compare March 1, 2023 12:47
@nateberkopec
Copy link
Member

Very nice 👏

@nateberkopec nateberkopec merged commit c8be369 into puma:master Mar 6, 2023
@Vuta Vuta deleted the warn_when_hooks_defined_in_single_mode branch March 6, 2023 16:09
if (@options[:workers] || 0) == 0
log_string =
"Warning: You specified code to run in a `#{hook_name}` block, " \
"but Puma is configured to run in cluster mode, " \
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

None yet

4 participants