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

Feature/add nohook script #29

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

Conversation

Startouf
Copy link

Ability to require the nvm tasks without adding automatically the nvm hooks

@koenpunt
Copy link
Owner

Can you give an example of why you don't want to automatically add the hooks?

@Startouf
Copy link
Author

Startouf commented May 21, 2018

We use capistrano roles to adapt our deployments to the needs of the (sub)-app we're deploying. For some roles, we don't need to compile any JS stuff, and since we don't use node/nvm, autoloading the script will be bad since currently hooks are ran on every server.
Many capistrano recipes often have a xx_roles variable that users can override to restrict the hooks to only those servers which have appropriate roles, but (and this is my personal preference) I'd rather have an explicit line in my deploy.rb file (or whatever stage configuration file) where I explicitely load hooks and/or choose where to add them in the capistrano callback chain.

@koenpunt
Copy link
Owner

If what you're saying is that this can be achieved by using roles, I don't see why we need another way of doing this. Also because like other Capistrano tasks, capistrano-nvm does have a nvm_roles variable, which you can use to control the execution.

@koenpunt
Copy link
Owner

Although you might be able to unregister the after hooks, or you could conditionally redefine the nvm:validate and nvm:map_bins with a no-op implementation.

@Startouf
Copy link
Author

Startouf commented Oct 2, 2018

I agree this follow the Rails guidelines of "Convention over configuration", but the nohooks scripts make it is easier to write clean code by splitting deployment into files that can be required only on some deploy configs.

For instance I have a Rails app that I can deploy very differently (eg Action Cable, old Rails app with HTML views, new API servers) depending on the config file.
I might be working to integrate React on my app (and such working with React-on-Rails in staging, which requires node) while I want to leave my production config free from that.
Having a nohook script allows me to require the proper config and have a clean "dry-run" overview of what's gonna happen.

I actually copied what I saw from the nice https://github.com/capistrano/passenger gem that has a nohook script in addition to an existing passenger_roles capistrano variable.

This also allows to really control the order in which hooks are added ad the order may be sensitive and not really easy to control from the capfile

Here is what my deploy folder looks like :

-- config/deploy
---- config/deploy/concerns
------ config/deploy/concerns/worker_sqs_deploy.rb 
------ config/deploy/concerns/worker_delayed_deploy.rb
------ config/deploy/concerns/mailer.rb 
------ config/deploy/concerns/api.rb
------ config/deploy/concerns/cable_passenger.rb
# old_app adds config config relevant to compiling assets and using React-on-Rails 
# (this one uses capistrano-nvm)
------ config/deploy/concerns/old_app.rb  

---- config/deploy/actioncable_staging.rb    # Requires cable_passenger
---- config/deploy/actioncable_production.rb  # Requires cable_passenger
---- config/deploy/api_production.rb          # Requires API + Mailer + Worker SQS + Old app concerns
---- config/deploy/api_staging.rb            # Requires API + Mailer + Worker delayed
---- config/deploy/xxx.rb

and in each of my deploy "stages" (production, staging, etc.) I require those files depending on what I need. Only the api_production environment requires nvm while other environments do not

@Startouf
Copy link
Author

Startouf commented Dec 12, 2018

Btw I've been using this in production for a few months now, no problem to report. It really does help since I'm updating the deploy configurations, and excluding node/yarn package construction lets me deploy much faster for those worker deploy scenarios where I don't need to compile JS

gem 'capistrano-nvm',
    git: 'https://github.com/Startouf/capistrano-nvm',
    branch: 'feature/add-nohook-script',
    require: false

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