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

Implements "nginx reload" handler #5

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

Implements "nginx reload" handler #5

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 23, 2017

For better high-availability, in some case it's more friendly to reload instead of restart.

That patch make possible the usage of notify: nginx reload.

@ghost
Copy link

ghost commented Feb 24, 2017

@dctremblay why not directly replace restart with reload? Is there a known case where restart is preferable to reload?

Also, as a side note: you introduce here a handler that isn't used in the role. What is your goal here, use that handler in another role? I think it's better practice, in ansible, not to rely on handlers from another role. If you need "nginx reload" in another role, add it to that role directly.

@ghost
Copy link
Author

ghost commented Feb 24, 2017

@hsoft If I do edit the pull request in order to completely replace restart with reload, would it be merged ?

@ghost
Copy link

ghost commented Feb 24, 2017

If you tell me that you've looked at nginx's doc and that you're confident that we won't break stuff by doing so, yes.

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.

0 participants