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.

Sign up for GitHub

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

Jump to bottom

add SSHKit::Backend::ConnectionPool#close_connections #285

Merged
merged 8 commits into from
Oct 26, 2015
Merged

add SSHKit::Backend::ConnectionPool#close_connections #285

merged 8 commits into from
Oct 26, 2015

Conversation

akm
Copy link
Contributor

@akm akm commented Sep 29, 2015

If sshkit is used not in capistrano but in a daemon, SSH connections are kept in CollectionPool. But there is no method to close them explicitly, so I added SSHKit::Backend::ConnectionPool#close_connections method to close them.

Suppressed net-ssh version < 3.0.0 to prevent installing error on ruby 1.9.2 and 1.9.3.

@akm akm changed the title [WIP] Add SSHKit::Backend::ConnectionPool#close_connections add SSHKit::Backend::ConnectionPool#close_connections Sep 29, 2015
@akm
Copy link
Contributor Author

akm commented Oct 1, 2015

@ppp0 I modified the dependency to use '~> 2.8' and it works. Thank you.

@ppp0
Copy link

ppp0 commented Oct 1, 2015

Excellent, I'm glad, thank you very much!

@leehambley
Copy link
Member

That seems lke a strange way to fix things, we don't want to get stuck on obsolete Net-ssh, I would have been happy to merge the original, but with this locking to an old version, I can't really. Happy to see the tests having passed.

@ppp0
Copy link

ppp0 commented Oct 1, 2015

@leehambley As far as I understand ~> 2.8 will install up to 2.9.x the last version requiring ruby 1.9 - Obsolete in what sense? 3.x is a major step forward and requires ruby >=2.0 which breaks my capistrano install and very likely a lot more gems that depend on ssh-kit.

To mitigate this, it would probably be good to tag a major version for ssh-kit, too, so that dependent libraries can reliably pin the version.

@leehambley
Copy link
Member

Thanks we should probably force ~> 2.8.x then, where x is whatever was most recently released. I saw that Net::SSH was making a move towards requiring Ruby 2.0, I could imagine us doing the same, I'd need to check what we do about Net::SSH, and what if any API changes over there warrant a forced minimum Ruby version.

@ppp0
Copy link

ppp0 commented Oct 1, 2015

I personally would stick with ~> 2.8 as it covers all releases up to 2.9.x
If you look at https://github.com/net-ssh/net-ssh/releases, there is only one 2.8.0 release. 3.x is the breaking point, ie when ruby >=2.0 is required

@akm
Copy link
Contributor Author

akm commented Oct 9, 2015

hi @leehambley

I think there are 6 options about this:

dependency ruby-1.9.2,1.9.3,2.0.0,2.1.2.2.2 ruby-2.0.0,2.1.2.2.2
net-ssh >= 2.8.0 [A] Failure because net-ssh-3.0.1 requires ruby >= 2.0.0 [D] Success
net-ssh ~> 2.8.x [B] Success (*1) [E] Success (*1)
net-ssh ~> 2.8 [C] Success [F] Success

*1 It doesn't support net-ssh-2.9.x and 2.10.x according to http://guides.rubygems.org/patterns/#declaring-dependencies

I recommend [C] because it is the most extensive support for ruby and net-ssh.

@mattbrictson
Copy link
Member

Ruby 1.9.3 has been removed from the Travis matrix, so the original form of this PR (without the net-ssh pin) should now pass CI checks.

@akm Perhaps you could rework this PR to remove the gemspec change?

@ppp0
Copy link

ppp0 commented Oct 26, 2015

@mattbrictson apologies, I was asking to pin the version because without, I will have a couple of things depending upon this that will break. Is it an unavoidable technical need to require ruby 2 here? Don't tests pass with versions pinned for ruby 2, too?

@akm
Copy link
Contributor Author

akm commented Oct 26, 2015

@mattbrictson I did. Is this alright?

leehambley added a commit that referenced this pull request Oct 26, 2015
@leehambley
Merge pull request #285 from groovenauts/add_close_connections_to_con…
dffcda1 
…nection_pool

add SSHKit::Backend::ConnectionPool#close_connections
@leehambley leehambley merged commit dffcda1 into capistrano:master Oct 26, 2015
@ppp0
Copy link

ppp0 commented Oct 26, 2015

support for ruby 1.9 was dropped

😢

@leehambley
Copy link
Member

support for ruby 1.9 was dropped

To clarify Net-SSH 3.x drops support for 1.9, and since we don't want to force people to use an older version of net-ssh (security, and our general policy on being liberal-to-a-fault with our dependency version requirements) - if you choose to lock Net-SSH at 2.x in your Gemfile, Cap and SSHKit will continue to work on 1.9 for the for seeable future. (at least 12 months past the Ruby 1.9 EOL). I'll document some of this in our README, or at least in the release notes.

Frankly we'd love to be able to use keyword arguments, and Module#prepend amongst other things. So eventually 1.9 support will have to go, but it's more a question of practicality.

@ppp0 ppp0 mentioned this pull request Oct 26, 2015
Merged
@mattbrictson
Copy link
Member

Also, the fact that Bundler resolves net-ssh to 3.x on Ruby 1.9 (thereby leading to an error) is due to a limitation of the current Bundler/rubygems. If all goes well it will be fixed soon.

@leehambley
Copy link
Member

Thanks for linking that issue Matt, so I feel vindicated in dropping the
1.9 suite, even if we don't explicitly require 1.9 yet. I'll add a note to
the SSHKIt and cap readmes
On 26 Oct 2015 2:17 p.m., "Matt Brictson" notifications@github.com wrote:

Also, the fact that Bundler resolves net-ssh to 3.x on Ruby 1.9 (thereby
leading to an error) is due to a limitation of the current
Bundler/rubygems. If all goes well it will be fixed soon
rubygems/bundler#4046.

—
Reply to this email directly or view it on GitHub
#285 (comment).

@njam njam mentioned this pull request Oct 26, 2015
Closed
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 14, 2015
Update ruby-net-ssh to 2.10.1.rc2.
cee0ba0 
## 1.8.1

  * Change license to MIT, thanks to all the patient contributors who gave
    their permissions.

## 1.8.0

  * add SSHKit::Backend::ConnectionPool#close_connections
    [PR #285](capistrano/sshkit#285)
    @akm
  * Clean up rubocop lint warnings
    [PR #275](capistrano/sshkit#275)
    @cshaffer
    * Prepend unused parameter names with an underscore
    * Prefer “safe assignment in condition”
    * Disambiguate regexp literals with parens
    * Prefer `sprintf` over `String#%`
    * No longer shadow `caller_line` variable in `DeprecationLogger`
    * Rescue `StandardError` instead of `Exception`
    * Remove useless `private` access modifier in `TestAbstract`
    * Disambiguate block operator with parens
    * Disambiguate between grouped expression and method params
    * Remove assertion in `TestHost#test_assert_hosts_compare_equal` that compares something with itself
  * Export environment variables and execute command in a subshell.
    [PR #273](capistrano/sshkit#273)
    @kuon
  * Introduce `log_command_start`, `log_command_data`, `log_command_exit` methods on `Formatter`
    [PR #257](capistrano/sshkit#257)
    @robd
    * Deprecate `@stdout` and `@stderr` accessors on `Command`
  * Add support for deprecation logging options.
    [README](README.md#deprecation-warnings),
    [PR #258](capistrano/sshkit#258)
    @robd
  * Quote environment variable values.
    [PR #250](capistrano/sshkit#250)
    @Sinjo - Chris Sinjakli
  * Simplified formatter hierarchy.
    [PR #248](capistrano/sshkit#248)
    @robd
    * `SimpleText` formatter now extends `Pretty`, rather than duplicating.
  * Hide ANSI color escape sequences when outputting to a file.
    [README](README.md#output-colors),
    [Issue #245](capistrano/sshkit#245),
    [PR #246](capistrano/sshkit#246)
    @robd
    * Now only color the output if it is associated with a tty,
      or the `SSHKIT_COLOR` environment variable is set.
  * Removed broken support for assigning an `IO` to the `output` config option.
    [Issue #243](capistrano/sshkit#243),
    [PR #244](capistrano/sshkit#244)
    @robd
    * Use `SSHKit.config.output = SSHKit::Formatter::SimpleText.new($stdin)` instead
  * Added support for `:interaction_handler` option on commands.
    [PR #234](capistrano/sshkit#234),
    [PR #242](capistrano/sshkit#242)
    @robd
  * Removed partially supported `TRACE` log level.
    [2aa7890](capistrano/sshkit@2aa7890)
    @robd
  * Add support for the `:strip` option to the `capture` method and strip by default on the `Local` backend.
    [PR #239](capistrano/sshkit#239),
    [PR #249](capistrano/sshkit#249)
    @robd
    * The `Local` backend now strips by default to be consistent with the `Netssh` one.
    * This reverses change [7d15a9a](capistrano/sshkit@7d15a9a) to the `Local` capture API to remove stripping by default.
    * If you require the raw, unstripped output, pass the `strip: false` option: `capture(:ls, strip: false)`
  * Simplified backend hierarchy.
    [PR #235](capistrano/sshkit#235),
    [PR #237](capistrano/sshkit#237)
    @robd
    * Moved duplicate implementations of `make`, `rake`, `test`, `capture`, `background` on to `Abstract` backend.
    * Backend implementations now only need to implement `execute_command`, `upload!` and `download!`
    * Removed `Printer` from backend hierarchy for `Local` and `Netssh` backends (they now just extend `Abstract`)
    * Removed unused `Net::SSH:LogLevelShim`
  * Removed dependency on the `colorize` gem. SSHKit now implements its own ANSI color logic, with no external dependencies. Note that SSHKit now only supports the `:bold` or plain modes. Other modes will be gracefully ignored. [#263](capistrano/sshkit#263)
  * New API for setting the formatter: `use_format`. This differs from `format=` in that it accepts options or arguments that will be passed to the formatter's constructor. The `format=` syntax will be deprecated in a future release. [#295](capistrano/sshkit#295)
  * SSHKit now immediately raises a `NameError` if you try to set a formatter that does not exist. [#295](capistrano/sshkit#295)
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 14, 2015
Update ruby-sshkit to 1.8.1.
e938525 
## 1.8.1

  * Change license to MIT, thanks to all the patient contributors who gave
    their permissions.

## 1.8.0

  * add SSHKit::Backend::ConnectionPool#close_connections
    [PR #285](capistrano/sshkit#285)
    @akm
  * Clean up rubocop lint warnings
    [PR #275](capistrano/sshkit#275)
    @cshaffer
    * Prepend unused parameter names with an underscore
    * Prefer “safe assignment in condition”
    * Disambiguate regexp literals with parens
    * Prefer `sprintf` over `String#%`
    * No longer shadow `caller_line` variable in `DeprecationLogger`
    * Rescue `StandardError` instead of `Exception`
    * Remove useless `private` access modifier in `TestAbstract`
    * Disambiguate block operator with parens
    * Disambiguate between grouped expression and method params
    * Remove assertion in `TestHost#test_assert_hosts_compare_equal` that compares something with itself
  * Export environment variables and execute command in a subshell.
    [PR #273](capistrano/sshkit#273)
    @kuon
  * Introduce `log_command_start`, `log_command_data`, `log_command_exit` methods on `Formatter`
    [PR #257](capistrano/sshkit#257)
    @robd
    * Deprecate `@stdout` and `@stderr` accessors on `Command`
  * Add support for deprecation logging options.
    [README](README.md#deprecation-warnings),
    [PR #258](capistrano/sshkit#258)
    @robd
  * Quote environment variable values.
    [PR #250](capistrano/sshkit#250)
    @Sinjo - Chris Sinjakli
  * Simplified formatter hierarchy.
    [PR #248](capistrano/sshkit#248)
    @robd
    * `SimpleText` formatter now extends `Pretty`, rather than duplicating.
  * Hide ANSI color escape sequences when outputting to a file.
    [README](README.md#output-colors),
    [Issue #245](capistrano/sshkit#245),
    [PR #246](capistrano/sshkit#246)
    @robd
    * Now only color the output if it is associated with a tty,
      or the `SSHKIT_COLOR` environment variable is set.
  * Removed broken support for assigning an `IO` to the `output` config option.
    [Issue #243](capistrano/sshkit#243),
    [PR #244](capistrano/sshkit#244)
    @robd
    * Use `SSHKit.config.output = SSHKit::Formatter::SimpleText.new($stdin)` instead
  * Added support for `:interaction_handler` option on commands.
    [PR #234](capistrano/sshkit#234),
    [PR #242](capistrano/sshkit#242)
    @robd
  * Removed partially supported `TRACE` log level.
    [2aa7890](capistrano/sshkit@2aa7890)
    @robd
  * Add support for the `:strip` option to the `capture` method and strip by default on the `Local` backend.
    [PR #239](capistrano/sshkit#239),
    [PR #249](capistrano/sshkit#249)
    @robd
    * The `Local` backend now strips by default to be consistent with the `Netssh` one.
    * This reverses change [7d15a9a](capistrano/sshkit@7d15a9a) to the `Local` capture API to remove stripping by default.
    * If you require the raw, unstripped output, pass the `strip: false` option: `capture(:ls, strip: false)`
  * Simplified backend hierarchy.
    [PR #235](capistrano/sshkit#235),
    [PR #237](capistrano/sshkit#237)
    @robd
    * Moved duplicate implementations of `make`, `rake`, `test`, `capture`, `background` on to `Abstract` backend.
    * Backend implementations now only need to implement `execute_command`, `upload!` and `download!`
    * Removed `Printer` from backend hierarchy for `Local` and `Netssh` backends (they now just extend `Abstract`)
    * Removed unused `Net::SSH:LogLevelShim`
  * Removed dependency on the `colorize` gem. SSHKit now implements its own ANSI color logic, with no external dependencies. Note that SSHKit now only supports the `:bold` or plain modes. Other modes will be gracefully ignored. [#263](capistrano/sshkit#263)
  * New API for setting the formatter: `use_format`. This differs from `format=` in that it accepts options or arguments that will be passed to the formatter's constructor. The `format=` syntax will be deprecated in a future release. [#295](capistrano/sshkit#295)
  * SSHKit now immediately raises a `NameError` if you try to set a formatter that does not exist. [#295](capistrano/sshkit#295)
@mfazekas mfazekas mentioned this pull request Apr 7, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@akm @ppp0 @leehambley @mattbrictson