Code review comment for lp:~axwalk/juju-core/gocryptossh-proxycommand

Revision history for this message
William Reade (fwereade) wrote :

LGTM, if you drop the local config check -- it adds an implicit
dependency to cmd, on an implementation detail of environs/config; and
we already have a conn open, so it shouldn't take too long to just ask
remotely. Ping me to discuss if you feel really strongly...

https://codereview.appspot.com/77300045/diff/20001/cmd/juju/ssh.go
File cmd/juju/ssh.go (right):

https://codereview.appspot.com/77300045/diff/20001/cmd/juju/ssh.go#newcode151
cmd/juju/ssh.go:151: // this avoids another API round-trip.
On 2014/03/27 13:50:54, axw wrote:
> On 2014/03/27 12:28:57, fwereade wrote:
> > Not valid, sadly: we can't depend on bootstrap config being
available.

> Updated to use bootstrap config if there, else go via API. Is this
valid?

Well, I don't love depending on the immutability, and we already have a
conn open. How much time do we save here, and how often?

https://codereview.appspot.com/77300045/

« Back to merge proposal