New Actor API

[joshgoebel]

This is a complete rewrite of most of the actors (especially SSH), transfer mechanism, and the Installer/Actor internal API... Passing the installers to the actors seems so logical to me... you could say separation of concern, blah... but I think it's the actors concern to install (and verify) things, so it makes sense to me to just pass them the installer or Verifyier and say "install [Apt]", "install [Transfer]", etc... and it's the only way (or the easy way?) to get to my next big feature of tuning the installers on a host-by-host basis to allow commands (and templates) to setup each host differently based on variables defined in the roles.

The actors know about the roles and hosts... so they need to be able to regenerate the installers (maybe later packages) dynamically per host (subbing variables, potentially changing logic, etc)... that's the big feature I'm missing that I think will make me mostly happy.

I'm submitting this for review and testing (not merging!)... I need to test it a bit more myself (likely beef up the tests) and I haven't done any testing of the Vlad code... it's just what looks correct based on what was there before.

Would love any thoughts and feedback. Anyone who can test that'd be great.

[joshgoebel]

I've mostly been testing the Cap and SSH code since that's easy and what I'm most familiar with.

[pschrammel]

I try to get my things running with your "new_api" branch:

If a command returns an error code with the ssh actor the package fails but sometimes it can fail (adduser can fail with a The user `xxxx' already exists.) to do sth because some parts of the package are already there.

[joshgoebel]

That is a failure... it should stop. Any deploy should stop if it hits a failure... add user should be written so that user existence isn't an issue.. though really your recipe should possibly create users in a separate step that can be validated.

[pschrammel]

But this breaks the current behaviour. And the level of granurality would very extreme. I mean: creating a directory that is already there (failure -> own package) creating a link that is already there (failure -> own package) .... I like the freedom of adding a runner to a package and a verifier so the verifier fails and the whole package is installed again. Some things fail (as things are already there) but this doesn't break the package if the final verification passes.

Am 13.12.2011 23:08, schrieb Josh Goebel:

That is a failure... it should stop. Any deploy should stop if it hits a failure... add user should be written so that user existence isn't an issue.. though really your recipe should possibly create users in a separate step that can be validated.

---

Reply to this email directly or view it on GitHub: https://github.com/crafterm/sprinkle/pull/41#issuecomment-3131281

[joshgoebel]

The current behavior with SSH was terrible (running sequential commands concurrently)... are you saying the current behavior (in master) with ALL other actors is as you describe? Or are you only speaking from your past experience with SSH?

Most commands have switches to avoid throwing an error. An error is an error. mkdir -p for example will silently ignore a directory if it already exists. Most commands do not throw errors for no reason. I don't believe errors should be ignored... recipes should be written to send proper commands. Since you can't tell what the type of error is unless you plan for the most boring of errors... a "directory already exists" error is entirely different than a "disk full" error.

[joshgoebel]

To create a link you would just use ln -f to force... which would ignore existing (or replace).

[joshgoebel]

I just checked master for Capistrano and it does abort on any error and I would say that is the correct behavior. Errors on only ignored during verification (so they can be re-raised as verification errors).

[joshgoebel]

The only logical behavior for deployment is to bail on an error and let the user examine the error and decide how to proceed. It's probably possible to hard code around this... runner "mkdir blah || true"... so even if mkdir failed true would still insure the result of the command was non-error.

[joshgoebel]

But again the best way is to use commands that don't throw errors and to write your packages in such a way that they can be reapplied without throwing errors.

[joshgoebel]

Rebases on latest master.

[dukejones]

I think @yyyc514 has a good point about failing out on exit codes other than 0. But, of course, leniency of error handling is largely a matter of taste. This would be a breaking change in that you would have to change your code to "mkdir -p" and "ln -s -f", but it seems like the correct behavior to my taste.

If it is not to your taste, @crafterm, I still think this branch has some very worthy changes to bring sprinkle to the next level. Just change the error handling.

[plexus]

+1 for this branch.

I started using it because it fixes https://github.com/crafterm/sprinkle/issues/10. I'm doing my own development on top of this branch.

[joshgoebel]

Done and done.