Nested "on" blocks should raise or warn

[kenn]

From #348 (comment)

AFAIK nested on blocks were never considered in the design. The fact that it works at all is really just coincidence, due to how the SSHKit DSL is mixed into all objects. IMO it should raise an exception. You can open an SSHKit issue.

My personal opinion is that we should actually support it (take intersection of all role sets in the call stack), but until then, we should raise or warn when we find a nested on blocks.

[leehambley]

AFAIK nested on blocks were never considered in the design. The fact that it works at all is really just coincidence, due to how the SSHKit DSL is mixed into all objects. IMO it should raise an exception. You can open an SSHKit issue.

It was simply never considered, I'd prefer to see them not supported. It's not only about the intersection of roles/hosts, but the on call is a unit of synchronization, and it'll be complex if they nest, I can't imagine how that would be expected to work.

[kenn]

It was simply never considered, I'd prefer to see them not supported. It's not only about the intersection of roles/hosts, but the on call is a unit of synchronization, and it'll be complex if they nest, I can't imagine how that would be expected to work.

I see your points, but I think the crux of this issue is that the behavior of on does not match the mental model of users - it looks like a declarative component in the nested task chain. In other words, on doesn't feel like a unit of synchronization, which could've been more suitable for a verb rather than a preposition. I admit that I'm very picky about the right name for a method. 😉

If we're able to change on to just keep hosts / roles inside the nested task context (define and set a dynamic instance variable on the caller object, which is a task lambda?), I see a couple of options here:

• Push down the point of synchronization to "verbs" like run, execute, etc. which is more semantically valid and intuitive place. create_command_and_execute will have SSHKit::Coordinator rather than SSHKit::Command, keeping reference to the hosts / roles set by outer on methods.
• Or, wild idea: We could just invoke new threads from inside another thread, as it does already - while I haven't fully thought out, it seems fine if we just nest the thread invocation and synchronization? In other words, people might already be doing that, even without noticing the nuances of internals.

If you still think it's not worth pursuing, let's just do warn / raise.

[mattbrictson]

Writing DSLs is hard. 😉

I think there is a possibility to make on work for nesting. A nested on could skip the entire coordination logic and function simply as a host filter, for example.

However, this would require a lot of code changes, and for the benefit of how many real-world use-cases? I'm not sure I see the cost-benefit equation working out.

[braindeaf]

I've just been poking at Capistrano to try and develop a provisioning recipe to kick off the setup of a new server and I found that nesting on groups in exactly what I needed. Something like

Here I would want to run the check:directories for example and it be limited to a subset of servers, however, since check:directories has it's own scope which just overwrites the original scope. I can't hijack and run it on a subset of servers. Damn.

on(:all, provisioning: true) do
  # on release_roles(:all) do
  #    execute :mkdir, "-p", shared_path, releases_path
  #. end
  invoke 'deploy:check:directories'
end

Just wondering how fun it would be to try making this work with nested chain of scopes.