capistrano / bundler

Bundler support for Capistrano 3.x
MIT License
219 stars 83 forks source link

Allow configuration of SSHKit options #93

Closed gondalez closed 7 years ago

gondalez commented 7 years ago

I found this useful to work around a rate limit with a custom gem repo :) There may be other applications.

e.g. set :bundle_task_ssh_kit_options, { in: :groups, limit: 50 }

capistrano-bot commented 7 years ago

Thanks for the PR! This project lacks automated tests, which makes reviewing and approving PRs somewhat difficult. Please make sure that your contribution has not broken backwards compatibility or introduced any risky changes.

Generated by :no_entry_sign: Danger

mattbrictson commented 7 years ago

Thanks for submitting this PR.

I understand the need for a workaround. This has come up before with Git, where Capistrano's built-in Git tasks can sometimes overload a private Git server when the number of servers is high enough.

Considering that this has been reported in at least two different places (Git and Bundler), I wonder if we should look for a more general solution?

What if there was a global setting that let you set an upper limit on parallelism in SSHKit as across the board? Would that work for you, or would you rather have fine-grained control over each task?

gondalez commented 7 years ago

Hi @mattbrictson :) A general solution makes sense to me.

In my case, though, an across-the board limit would add too much time to other tasks that are fine to run in parallel.

Do you think a global setting as well as per-task overrides could work?

It's pie in the sky but I imagine something like this could work:

set :sshkit_config, {
  "global"=> {in: :groups, limit: 100},
  "bundler:install" => {in: :groups, limit: 50},
  "git:check" => {in: :groups, limit 10},
}
mattbrictson commented 7 years ago

Unfortunately it isn't that straightforward. Parallelization in SSHKit works at the granularity of the on block, not at the task level. There can be many on blocks per task, and on blocks can even exist outside of tasks. Not to mention, an on block has no visibility into what task triggered it. So a task-based configuration will not work.

Perhaps there is another approach I'm not seeing, but I think we are limited to doing something globally, or we have to monkey-patch/rewrite every specific task that we want to affect. Neither option seems very appealing.

I don't want to set a precedent for a task-specific configuration variable (e.g. bundle_task_ssh_kit_options). I'd rather wait to see if someone comes up with a more general-purpose solution. Unfortunately I think that people in your situation who are hitting edge cases with lots of servers will just need to customize/patch their deployment scripts in the meantime.

mattbrictson commented 7 years ago

I've decided against merging this for now. Thanks for the PR and the comments! I've opened #94 to continue the discussion.

gondalez commented 7 years ago

I completely understand @mattbrictson; a slippery slope in terms of cohesiveness. In all honesty I think the correct solution for us is to rethink our deploys and move to running the bundle once before deploying the output.

This workaround will buy us some time for that non-trivial implementation.

Thanks for the feedback 👍