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

local cmd modifications based on remote cmd needs/work #14

Merged
merged 1 commit into from
Jul 1, 2016

Conversation

kellyredding
Copy link
Member

This is a set of changes to the local cmd objects based on R&D
and implementation work on remote cmds. There are a couple of
small things here (however, the main goal of this is to keep the
implementations as similar as possible):

  • have the base cmd take the local cmd/spy class and create an
    instance. This has no specific value to local cmds, but remote
    cmds needed this API so I'm updating local cmds to match.
  • track the cmd str and to_s manually (instead of demetering from
    the Scmd). Again, no specific value here, but the remote cmd
    needed this so I want the local to match. Also, this decouples
    slightly from the Scmd.

Finally, this removes the run! method from local cmds. This
issue is that we were relying on Scmd to error in this case. This
means the error was previously an Scmd error with a backtrace
buried in the local cmd internals. A better implementation is to
handle this logic in the task (where the cmd! api is actually
needed). This switches to just implementing/calling run on the
local cmd and runner. For the task's cmd! method, the run meth
is called and then if the cmd was not successful, a custom task
exception is raised with the caller of the cmd! method as the
backtrace. This results in better errors for the users. Also,
this is handy b/c it lines up with the remote cmd api/handling.
See PR 13 for reference.

See #13 for reference.

@jcredding ready for review.

@jcredding
Copy link
Member

@kellyredding - I like not relying on scmd to raise the error for cmd! and instead handling it in dk. Good catch on that. I also like that it simplified the local cmd spy, removing the bang call/called methods.

Looks good 💥

This is a set of changes to the local cmd objects based on R&D
and implementation work on remote cmds.  There are a couple of
small things here (however, the main goal of this is to keep the
implementations as similar as possible):

* have the base cmd take the local cmd/spy *class* and create an
  instance.  This has no specific value to local cmds, but remote
  cmds needed this API so I'm updating local cmds to match.
* track the cmd str and to_s manually (instead of demetering from
  the Scmd).  Again, no specific value here, but the remote cmd
  needed this so I want the local to match.  Also, this decouples
  slightly from the Scmd.

Finally, this removes the `run!` method from local cmds.  This
issue is that we were relying on Scmd to error in this case.  This
means the error was previously an Scmd error with a backtrace
buried in the local cmd internals.  A better implementation is to
handle this logic in the task (where the `cmd!` api is actually
needed).  This switches to just implementing/calling `run` on the
local cmd and runner.  For the task's `cmd!` method, the run meth
is called and then if the cmd was not successful, a custom task
exception is raised with the caller of the `cmd!` method as the
backtrace.  This results in better errors for the users.  Also,
this is handy b/c it lines up with the remote cmd api/handling.
See PR 13 for reference.
@kellyredding
Copy link
Member Author

@jcredding thx man - I like how the cmd stuff is coming together. I feel like there are some nice abstractions and reuse in play. The remote stuff wasn't too hard to layer on either.

@kellyredding kellyredding merged commit a1fff4d into master Jul 1, 2016
@kellyredding kellyredding deleted the kr-local-cmd-mods branch July 1, 2016 17:06
kellyredding added a commit that referenced this pull request Aug 16, 2016
* (hotfix) super minor gemspec desc/summary cleanup f3b3d21
* first pass on Task mixin; basic run login and desc DSL #1
* add the base Runner; add params-related task helper methods #2
* `run_task` task helper method #3
* stringify runner params #4
* setup the runner; break out runner API for running sub-tasks #5
* add task test helpers for building test runners and test tasks #6
* task callback DSL #7
* `halt` task helper method #8
* log task helper methods #9
* local cmd and spy objects #10
* rework test runner test task API #11
* add local cmd handling API on tasks and runners #12
* remote cmd and spy objects #13
* local cmd modifications based on remote cmd needs/work #14
* add remote cmd handling API on tasks and runners #15
* add ssh "markup" to RemoteCmd LocalCmd strings #16
* switch RemoteCmd to use custom OutputLine that includes host info #17
* first pass at Task related README content #18
* add Config class; use to configure/initialize Dk #19
* add a ConfigRunner and have the dk/dry runners subclass it #21
* add `set_param` config method and use config params in config runners #22
* commonize the `set_param` logic between the Config and Runner #23
* add a task callback API to the Config #24
* add ssh dsl methods to the Config #25
* have Runner use Config's ssh opts; have Task fall back to Runner ssh opts #26
* (hotfix) README tweak from PR 26 1ac39f0
* add a `task` DSL method to Config for adding "callable" tasks #27
* add host-specific ssh args to Config DSL; honor when making ssh cmds #28
* build a Logsly logger from a config and use on the live/dry runners #29
* add first-pass README docs for the config DSL #30
* basic CLI with runner flags and list tasks options #31
* first pass CLI README docs #32
* (hotfix) manually convert Pathname to String in CLI tests e5a09c7
* (hotfix) make bin/dk executable 53dcc8d
* add testing helper methods for Task ssh hosts and callbacks #33
* (hotfix) add `task_callback` factory 716f52c
* put Scmd in test while running the test runner #34
* (hotfix) force ssh hosts values to an Array 3d6fcaa
* add `ssh_cmd_str` Task helper method #35
* (hotfix) add `ssh_cmd_str` test helper 34b13fb
* change Config to store task callbacks #36
* add Task DSL method to ensure tasks run only once #37
* add `param?` Task helper method #38
* Pass stdin to local and remote cmds #39
* Fix `cmd!` wrong number of args error #40
* Rework ssh stubbing to avoid having to know ssh opts #41
* add helper methods to add task callbacks from tasks #42
* (hotfix) have local/remote cmd spies know if they are ssh or not 1408b20
* implement listing tasks w/ descs with CLI --help and -T options #45
* (hotfix) fix tests for ruby2; make Remote complain if given `nil` hosts 93be0fd
* (hotfix) switch to logging cmd/ssh cmd strs at the debug level 7517591
* add `--verbose` CLI flag #46
* add `:dry_tree_run` cmd/ssh opt to run cmds w/ the dry/tree runners #47
* have the TreeRunner output the tasks that were run #48
* make the `run_only_once` task behavior more robust #49
* track task callbacks using a CallbackSet #50
* have tasks log when they run #53
* style up the user task log messages #54
* Add storing dry/tree runner stubs to `Config` #52
* style up cmd/ssh log output; match styles in task/other logs #55
* Add `Dk::HasTheStubs` mixin, use in test and dry runner #51
* add debug log entries at the start of new CLI runs #57
* Update `DryRunner`, apply dry tree stubs from `Config` on init #56
* add ansi styled log messages #61
* pre-check all task names before running the tasks #63
* Log a task only around its run, not around its callbacks #62
* (hotfix) have the Task `log_*` methods task optional style args e69074d
* (hotfix) log an empty line before and *after* task runs 683d796
* Hotfix: Fix `pretty_run_time` displaying minutes and seconds as floats 117fff6
* update non-verbose logs to no longer show task starts/ends #64
* (hotfix) have remote cmds expose their stdout and stderr
* tweak the task run start/end log buffer identifiers #65
* add a custom exception for notifying the user of problems #68
* update stubs API to optionally take proc values #66
* add `try_param` task helper and hash method #67
* add host info to ssh cmd output lines logs #69
* (hotfix) time/log local cmd run time like we do for ssh cmds 155977a
* (hotfix) switch to "dk" name for default task config file 18c7e32
* (hotfix) tweak time displays to cross over at 1.5s b3273f6
* (hotfix) update to the latest assert 17c2261
* (hotfix) add README blurb linking Dk gems 4f4a151
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