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

Simplify backend hierarchy #237

Merged
merged 18 commits into from
Apr 26, 2015
Merged

Simplify backend hierarchy #237

merged 18 commits into from
Apr 26, 2015

Conversation

robd
Copy link
Contributor

@robd robd commented Apr 26, 2015

This PR also came out of my explorations for implementing #234. Specifically, I was looking at the possibility of supporting interactive sessions by subclassing, rather than adding another option to Command. I was looking to see where it was necessary to hook Command.new and as far as I can tell execute in CommandHelper wasn't actually called because the the backends which included CommandHelper overrode execute. This led me to look into the backend hierarchy and try to simplify things a bit.

This PR includes 3 main changes:

  1. Moved make, rake, test, capture and background on to the Abstract backend. This was to remove duplicate implementations on Local and Netssh backends and remove redundant inclusion of CommandHelper in Netssh.
  2. Moved the following duplicate command creation code from the Local, Netssh, Skipper and Printer backends into the Abstract backend by implementing execute in Abstract and introducing the execute_command abstract method on the subclasses instead.
def execute(args)
  command(*args).tap do |cmd|
    # Execute the command
  end
end
  1. Removed Printer from the Local and Netssh hierarchies since the Printer method implementations were overridden by the subclasses. As part of this, I also moved the output method from Printer up to Abstract, since it is needed by the info(messages), debug(messages) etc methods on Abstract. I also implemented run on Abstract and removed the duplicate implementations from the subclasses.

robd added 18 commits April 25, 2015 15:43
@robd
Inlined CommandHelper make and rake methods into Abstract backend
aa69eb2 
The execute method on CommandHelper was overridden by the execute methods on Printer or Netssh, so was never used. The map method on CommandHelper was never called.
@robd
Moved duplicate implementations of backend.run on to abstract backend
2d069f9
@robd
Moved output method from Printer to Abstract backend
94dc6b1 
The debug, info, warning etc methods on Abstract all depend on the output method, so it seems sensible to define it in Abstract too.
@robd
Removed Printer from Local and Netssh backend hierarchy
4c41dde
@robd
Fixed Abstract.{execute,test,capture} method signatures
1ab4121 
Brought these into line with implementations on the Local and Netssh backends. Reworked TestPrinter to test the Abstract backend instead.
@robd
Inlined confusingly named printer method
7322da6 
This was a copy and paste from the TestPrinter unit test
@robd
Pulled duplicate test methods from Local/Netssh backends up to Abstract
26bfa3d
@robd
Pulled background method from Netssh backend up to Abstract
e970f6c
@robd
Introduced execute_command on backends
0a8122f 
Pulled duplicated creation and returning of commands up to Abstract backend
@robd
Pulled duplicated capture method up to Abstract backend
d24ea78
@robd
Reduce indentation execute_command methods
64b5901
@robd
Removed duplication in extracting command options
8f27e29
@robd
Added tests for Abstract backend methods: rake,make,execute,test,capture
654a7ae
@robd
Added stub implementations for abstract methods on Abstract backend
d5e6ad4 
Stub methods throw MethodUnavailableError to be consistent with previous behaviour
@robd
Removed unused Net::SSH:LogLevelShim
a5de975 
I believe this is now implemented on the Abstract backend
@robd
Added notes to changelog
482e500
@robd
Removed unused invoke method from Skipper backend
a9fb155
@robd
Removed capture method from Printer
58db2d1
leehambley added a commit that referenced this pull request Apr 26, 2015
@leehambley
Merge pull request #237 from robd/simplify-backend-hierarchy
81aa347 
Simplify backend hierarchy
@leehambley leehambley merged commit 81aa347 into capistrano:master Apr 26, 2015
@leehambley
Copy link
Member

Thanks Rob :)

@robd
Copy link
Contributor Author

robd commented Apr 26, 2015

No probs - thanks for merging this so quickly. I hope to be able to actually implement #234 now!

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)
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.
2 participants
@robd @leehambley