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

Unicorn nginx task opts #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Unicorn nginx task opts #56

wants to merge 3 commits into from

Conversation

amkirwan
Copy link
Contributor

@amkirwan amkirwan commented Mar 7, 2015

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
Copy link
Contributor

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

@amkirwan
Copy link
Contributor Author

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
Copy link
Contributor

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 [email protected]
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
#56 (comment)
.

@amkirwan
Copy link
Contributor Author

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
Copy link

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
Copy link
Collaborator

Preen commented Apr 20, 2017

I'm positive to the option of having this.

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.

4 participants