Implement unit-get command

Implement unit-get command

Note that "smart" output formatting no longer exists; we default to yaml.

Also consolidated common test code.

https://code.launchpad.net/~fwereade/juju/go-unit-get-command/+merge/104537

Requires: https://code.launchpad.net/~fwereade/juju/go-config-get-command-flags/+merge/104410

(do not edit description out of merge proposal)

[fwereade, 2012-05-03 11:56:05]

Please take a look.

[fwereade, 2012-05-03 12:48:24]

Please take a look.

[niemeyer, 2012-05-03 19:21:10]

LGTM. Just trivial details..

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/context.go
File cmd/jujuc/server/context.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/context.go#...
cmd/jujuc/server/context.go:27: // checkUnitState returns an error if ctx does
not have non-zero State and
It's not clear to me how it's checking the unit state. It seems to check the
(Client)Context. Isn't check() a better name?

Also: s/does not have non-zero State and/has a nil state or/

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go
File cmd/jujuc/server/unit-get.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:13: ctx *Context
If you're happy with the other review, this should be as well.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:14: out resultWriter
This sounds like something that should remain private, though.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:15: Arg string
Arg doesn't sound right. Key?

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:32: c.out.addFlags(f, "smart", defaultConverters)
What's the default for these commands today? smart doesn't sound right. This
should be "json" or "yaml".

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/util_test.go
File cmd/jujuc/server/util_test.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/util_test.g...
cmd/jujuc/server/util_test.go:47: return w.(*bytes.Buffer).String()
Same comment as the previous review.

[fwereade, 2012-05-03 22:54:03]

Please take a look.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/context.go
File cmd/jujuc/server/context.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/context.go#...
cmd/jujuc/server/context.go:27: // checkUnitState returns an error if ctx does
not have non-zero State and
On 2012/05/03 19:21:10, niemeyer wrote:
> It's not clear to me how it's checking the unit state. It seems to check the
> (Client)Context. Isn't check() a better name?
> 
> Also: s/does not have non-zero State and/has a nil state or/

I'm expecting at some stage to checkRelationState as well -- ie, verify that it
has the data needed by (say) relation-get, as opposed to in this case checking
validity for unit-get/config-get/open-port.

Done for now all the same, it's speculative :).

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go
File cmd/jujuc/server/unit-get.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:13: ctx *Context
On 2012/05/03 19:21:10, niemeyer wrote:
> If you're happy with the other review, this should be as well.

Done.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:14: out resultWriter
On 2012/05/03 19:21:10, niemeyer wrote:
> This sounds like something that should remain private, though.

Agreed.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:15: Arg string
On 2012/05/03 19:21:10, niemeyer wrote:
> Arg doesn't sound right. Key?

Done.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/unit-get.go...
cmd/jujuc/server/unit-get.go:32: c.out.addFlags(f, "smart", defaultConverters)
On 2012/05/03 19:21:10, niemeyer wrote:
> What's the default for these commands today? smart doesn't sound right. This
> should be "json" or "yaml".

Sad to say, it is indeed "smart" [0]; and we don't have a "yaml". I agree it
*should* be a sane/consistent format, but compatibility yada yada. Can we
discuss this tomorrow? (hmm, if we coincide...)

[0] juju/hooks/cli.py:75

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/util_test.go
File cmd/jujuc/server/util_test.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/util_test.g...
cmd/jujuc/server/util_test.go:47: return w.(*bytes.Buffer).String()
On 2012/05/03 19:21:10, niemeyer wrote:
> Same comment as the previous review.

Same questions as the previous review :).

[fwereade, 2012-05-04 08:10:43]

Please take a look.

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/util_test.go
File cmd/jujuc/server/util_test.go (right):

https://codereview.appspot.com/6174051/diff/3001/cmd/jujuc/server/util_test.g...
cmd/jujuc/server/util_test.go:47: return w.(*bytes.Buffer).String()
On 2012/05/03 22:54:03, fwereade wrote:
> On 2012/05/03 19:21:10, niemeyer wrote:
> > Same comment as the previous review.
> 
> Same questions as the previous review :).

Done.

[fwereade, 2012-05-09 19:39:16]

*** Submitted:

Implement unit-get command

Note that "smart" output formatting no longer exists; we default to yaml.

Also consolidated common test code.

R=niemeyer
CC=
https://codereview.appspot.com/6174051