Merge lp://staging/~jimbaker/pyjuju/ssh-passthrough into lp://staging/pyjuju

[0] trivial

+ "[passthru ssh options] unit_or_machine [command]"),

s/passthru/passthrough

[1] trivial

+ # workding with ssh, for example), so consume the rest and

s/workding/working

[2] less trivial

I didn't see any way to handle potential conflicts in args (specifically -o ControlMaster/ControlPath, which I think are the only ones we need to worry about at the moment). Not really a big deal, but something it might be worth logging a "low" bug against, to make it clear that the limitation is known but not a priority?

anyway, +1

> [0] trivial
>
> + "[passthru ssh options] unit_or_machine [command]"),
>
> s/passthru/passthrough

Thanks, let's keep it consistent. (Both seem to be used, but passthrough more so. Or perhaps pass-through.)

>
> [1] trivial
>
> + # workding with ssh, for example), so consume the rest and
>
> s/workding/working

Ack

>
> [2] less trivial
>
> I didn't see any way to handle potential conflicts in args (specifically -o
> ControlMaster/ControlPath, which I think are the only ones we need to worry
> about at the moment). Not really a big deal, but something it might be worth
> logging a "low" bug against, to make it clear that the limitation is known but
> not a priority?

Indeed, it's possible for the user to escape the walled garden and use ssh flags in bad ways as well as good. Passthrough flags are presented later to ssh, and can thereby override juju's default usage. In the particular example you cite, this enables the user to know what is best for a given scenario. For example, multiplexing a connection would not be so ideal if there's significant data transfer. Until now this was a moot point, but with tunnel support, this need could actually arise, and it can be simply solved by passing in -o "ControlMaster no".

I don't think it's a low bug, just simply a matter of adding to the documentation of this command.

>
> anyway, +1

Thanks!

> I don't think it's a low bug, just simply a matter of adding to the
> documentation of this command.

To be precise: it should be fixed in the context of this branch.

>
> > I don't think it's a low bug, just simply a matter of adding to the
> > documentation of this command.
>
> To be precise: it should be fixed in the context of this branch.

irc discussion:

<jimbaker> fwereade, i think i'm going to change my mind on the doc stuff related to juju ssh. the command itself is not documented. really we need to incorporate more detailed docs on each subcommand in juju
<jimbaker> fwereade, so that should be done in a separate branch/bug
<fwereade> jimbaker, sounds sensible to me

Nice work jim. lgtm, a minor

[0]

+def parse_passthrough_args(args, flags_taking_arg=frozenset()):

the frozenset seems like overkill when an empty tuple suffices.

[1]

re the same function.

Is supporting long options just a matter of an additional conditional
clause for peeked.startswith("--") ? If so it seems better to just
implement support for it now.

[2] re the conflict william brought up, experimenting with the ssh cli it doesn't actually appear to be a problem in practice the user specified option will take precedence as its specified last.

> Nice work jim. lgtm, a minor
>
> [0]
>
> +def parse_passthrough_args(args, flags_taking_arg=frozenset()):
>
> the frozenset seems like overkill when an empty tuple suffices.

Sure, definite overkill.

>
> [1]
>
> re the same function.
>
> Is supporting long options just a matter of an additional conditional
> clause for peeked.startswith("--") ? If so it seems better to just
> implement support for it now.

It's not quite so simple, and so far the universe of commands that need this passthrough support is as far as I know is just ssh/scp. So I'd suggest punting that support until we identify them.
>
>
> [2] re the conflict william brought up, experimenting with the ssh cli it
> doesn't actually appear to be a problem in practice the user specified option
> will take precedence as its specified last.

My experience too.