capistrano-plugins / capistrano-unicorn-nginx

Capistrano tasks for automatic and sensible unicorn + nginx configuration
MIT License
175 stars 81 forks source link

Unicorn nginx task opts #56

Open amkirwan opened 9 years ago

amkirwan commented 9 years ago

Created some options to allow the user to control what tasks are run when $ bundle exec cap production setup and $ bundle exec cap production deploy. With these options if the user for example wants to run all the tasks in this plugin except for having it create the nginx config they can set the option set :setup_nginx, false to skip creating the nginx config.

rhomeister commented 9 years ago

Could you explain the rationale behind this PR? In which cases would you not want to reload nginx?

amkirwan commented 9 years ago

If you are using Rails to only serve an API then any updates to your app wouldn't require nginx to reload as it's only role would really be to forward requests to the application server. There would be no need for any static files it to serve.

The point is to give fine grain control over what tasks are run with sensible defaults. I could certainly see someone wanting to use this cap task and have it not manage nginx config files or restarts but use it for managing the unicorn. This would definitely be true in an app that has say a Rails API backend and is using a JS framework for the front end.

rhomeister commented 9 years ago

Hi Anthony,

I'm not using nginx for serving static files, but the reload takes so little time that there is 0 downtime. Adding more code to prevent a problem that IMHO doesn't exist will only decrease the maintainability of this gem.

Ruben

On 17 September 2015 at 18:55, Anthony Kirwan notifications@github.com wrote:

If you are using Rails to only serve an API then any updates to your app wouldn't require nginx to reload as it's only role would really be to forward requests to the application server. There would be no need for any static files it to serve.

The point is to give fine grain control over what tasks are run with sensible defaults. I could certainly see someone wanting to use this cap task and have it not manage nginx config files or restarts but use it for managing the unicorn. This would definitely be true in an app that has say a Rails API backend and is using a JS framework for the front end.

— Reply to this email directly or view it on GitHub https://github.com/capistrano-plugins/capistrano-unicorn-nginx/pull/56#issuecomment-141147167 .

amkirwan commented 9 years ago

I suppose it depends on how many virtual sites your nginx server is hosting. That is why I added the option so end users could make there own decision on that. I see it as really minimally making the code base anymore complex since that one particular feature is two lines of code. One for setting the default value of restarting nginx to true and the other to check what the value is. The default is set to restart NGINX so most end users wouldn't even need to do anything.

Plus that is only one small part of the pull request. I've used several of the other features I've added several times in different apps. I have JS apps that need to restart the NGINX server on deploy but don't need to restart the API's Unicorn server or create the NGINX or Unicorn config files.

I see the options as giving the developer the flexibility to use this Capistrano task as they need to for their particular setup with a minimal amount of additional code. In addition the defaults are set to values so that nothing is changed in how this task works unless the developer changes one of the settings explicitly.

kanmeiban commented 7 years ago

Hi Ruben,

I need this option so badly that I am willing to become maintainer of this superb gem. I am tired of my half-baked deploy solutions and seeing the fine job you did it simply must be carried on.

Sava

Preen commented 7 years ago

I'm positive to the option of having this.