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

Add option to disable management of rundir #674

Merged
merged 1 commit into from Jan 30, 2023
Merged

Conversation

tmanninger
Copy link
Contributor

@tmanninger tmanninger commented Dec 13, 2022

I am using php7.4 from sury on debian 10.
After every server reboot, this module creates the /var/run/php-fpm directory (which is not used by php-fpm) and triggers a reload, which resets all open connections to the php-fpm daemon.

This PR added an option to disable the managing of the /var/run/php-fpm directory

@kenyon
Copy link
Member

kenyon commented Dec 13, 2022

Why are we managing this directory if it's not used? Maybe there is a better solution here, like making it conditional on some version of package or OS?

@tmanninger
Copy link
Contributor Author

We are using /var/run/ and i wouldn't manage this directory with the puppet-php module.

@tmanninger
Copy link
Contributor Author

It there no way to merge this MR?
I am using the PHP package from packages.sury.org, and i don't know, which OS versions and PHP versions are affected.
I think the best way is it to make it configureable.

Copy link
Member

@kenyon kenyon left a comment

Choose a reason for hiding this comment

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

Seems OK to me. We'll want to rebase when merging this to eliminate the merge commit.

@tmanninger
Copy link
Contributor Author

@kenyon i removed the merge commit

@tmanninger
Copy link
Contributor Author

@smortex i added the documentation

@tmanninger
Copy link
Contributor Author

@smortex is everything fine?

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

This feel weird, but I don't know what this directory is used for and the default behavior is unchanged so LGTM 🤷.

@smortex
Copy link
Member

smortex commented Jan 30, 2023

CI failure seems unrelated, merging

@smortex smortex merged commit 594c7bd into voxpupuli:master Jan 30, 2023
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.

None yet

3 participants