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 an option to disable monit configuration #188

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

Conversation

kraft001
Copy link

Solves #181
Allows a user to skip the monit configuration step if monit was already configured on the server

@Tensho
Copy link
Contributor

Tensho commented Apr 3, 2018

This controversy to #193. IMO, sidekiq monit conf MUST be re-upload on every deploy automatically. Suppose you changed some sidekiq options that should be reflected in monit start command inside monit config file, e.g. sidekiq_concurrency or sidekiq_logfile.

@kraft001
Copy link
Author

kraft001 commented Apr 3, 2018

@Tensho I completely agree with your point, it's valid when a developer has access to monit config files.
But in our case monit is managed by sysadmins who built their own Puppet scripts, and developers cannot change those scripts. More options don't hurt but help developers to handle such cases.

@Tensho
Copy link
Contributor

Tensho commented Apr 3, 2018

@kraft001 ah, I see what you mean. In such a case it makes sense, even though sysops should reflect (sync) any configuration changes developers introduced by hand in this case. Does capistrano-unicorn or capistrano-puma have the similar configuration option?

@kraft001
Copy link
Author

kraft001 commented Apr 3, 2018

@Tensho Neither of the mentioned gems has such an option

@Tensho
Copy link
Contributor

Tensho commented Apr 4, 2018

@kraft001 🤔 Maybe conventionally deploy should be run from Puppet system either... I don't see any significant drawbacks incurring this config option. Could you please rebase the branch?

@seuros, What do you think on the subject?

@kraft001 kraft001 force-pushed the disable_monit_configuration branch from 49a6282 to adcef17 Compare April 4, 2018 06:42
@kraft001 kraft001 force-pushed the disable_monit_configuration branch from adcef17 to d078289 Compare April 4, 2018 06:46
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.

2 participants