Add a get-environment command.

Add a get-environment command.

Be able to get environment values from state.  You can ask for either a single
value, or get the whole lot formatted how you like (well within reason).

set-environment coming next.

https://code.launchpad.net/~thumper/juju-core/get-environment/+merge/157968

(do not edit description out of merge proposal)

[thumper, 2013-04-09 22:59:48]

Please take a look.

[thumper, 2013-04-09 23:02:28]

https://codereview.appspot.com/8597045/diff/1/cmd/juju/main_test.go
File cmd/juju/main_test.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/main_test.go#newcode219
cmd/juju/main_test.go:219: "get-env", // alias for set-environment
Fixed the alias comment in the branch already, but didn't want to push up a new
change set.

[dave_cheney.net, 2013-04-10 01:40:45]

LGTM. Thank you.

[dimitern, 2013-04-10 09:56:11]

Looks good, but I have some questions.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go
File cmd/juju/environment.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode13
cmd/juju/environment.go:13: type GetEnvironmentCommand struct {
Why not just juju get instead?

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode21
cmd/juju/environment.go:21: for the environment are output using the selected
formatter.
Is the formatter handled by the base command? I cannot see it specifically
formatting the output here.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode75
cmd/juju/environment.go:75: return fmt.Errorf("Environment key %q not found in
%q environment.", c.key, config.Name())
s/Environment//

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment_test.go
File cmd/juju/environment_test.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment_test.go#ne...
cmd/juju/environment_test.go:33: },
how about a test with all values? maybe even one with different formats?

[dimitern, 2013-04-10 11:27:45]

After speaking with fwereade, I drop my comment about the name.

On 2013/04/10 09:56:11, dimitern wrote:
> Looks good, but I have some questions.
> 
> https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go
> File cmd/juju/environment.go (right):
> 
>
https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode13
> cmd/juju/environment.go:13: type GetEnvironmentCommand struct {
> Why not just juju get instead?
>

[fwereade, 2013-04-10 11:33:25]

LGTM modulo ZeroOrOneArgs suggestion and more-detailed testing.

https://codereview.appspot.com/8597045/diff/1/cmd/cmd.go
File cmd/cmd.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/cmd.go#newcode49
cmd/cmd.go:49: func (c *CommandBase) ZeroOrOneArgs(args []string) (string,
error) {
I don't really think this deserves to live on the type. Free func like
CheckEmpty?

https://codereview.appspot.com/8597045/diff/1/cmd/cmd.go#newcode54
cmd/cmd.go:54: return result, CheckEmpty(args)
I generally prefer to return definite zero values with errors.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go
File cmd/juju/environment.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode13
cmd/juju/environment.go:13: type GetEnvironmentCommand struct {
On 2013/04/10 09:56:11, dimitern wrote:
> Why not just juju get instead?

Different realms, potentially different options and formatting. Better to keep
them separate, as agreed in Atlanta.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode21
cmd/juju/environment.go:21: for the environment are output using the selected
formatter.
On 2013/04/10 09:56:11, dimitern wrote:
> Is the formatter handled by the base command? I cannot see it specifically
> formatting the output here.

c.out.AddFlags, c.out.Write

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode43
cmd/juju/environment.go:43: // TODO(thumper) --private to also include private.
"to exclude private settings when unset" or something, just to be explicit?

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment_test.go
File cmd/juju/environment_test.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment_test.go#ne...
cmd/juju/environment_test.go:33: },
On 2013/04/10 09:56:11, dimitern wrote:
> how about a test with all values? maybe even one with different formats?

+1 for all values, +0 for multiple formats -- the flag should be tested, but we
don't need to test its behaviour exhaustively.

[thumper, 2013-04-10 22:36:55]

Updated the branch, but I'm going to land this at the same time as
set-environment so we get the pair together.

https://codereview.appspot.com/8597045/diff/1/cmd/cmd.go
File cmd/cmd.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/cmd.go#newcode49
cmd/cmd.go:49: func (c *CommandBase) ZeroOrOneArgs(args []string) (string,
error) {
On 2013/04/10 11:33:25, fwereade wrote:
> I don't really think this deserves to live on the type. Free func like
> CheckEmpty?

Fair enough.

https://codereview.appspot.com/8597045/diff/1/cmd/cmd.go#newcode54
cmd/cmd.go:54: return result, CheckEmpty(args)
On 2013/04/10 11:33:25, fwereade wrote:
> I generally prefer to return definite zero values with errors.

Done.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go
File cmd/juju/environment.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode43
cmd/juju/environment.go:43: // TODO(thumper) --private to also include private.
On 2013/04/10 11:33:25, fwereade wrote:
> "to exclude private settings when unset" or something, just to be explicit?

This was initially a consideration about whether to include private values by
default, and that we could have an option to include them. In the end I just
implemented the "get everything" option.  We can always add a --private back in
if desired at a later time.  I'll just remove the comment.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment.go#newcode75
cmd/juju/environment.go:75: return fmt.Errorf("Environment key %q not found in
%q environment.", c.key, config.Name())
On 2013/04/10 09:56:11, dimitern wrote:
> s/Environment//

Done.

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment_test.go
File cmd/juju/environment_test.go (right):

https://codereview.appspot.com/8597045/diff/1/cmd/juju/environment_test.go#ne...
cmd/juju/environment_test.go:33: },
On 2013/04/10 11:33:25, fwereade wrote:
> On 2013/04/10 09:56:11, dimitern wrote:
> > how about a test with all values? maybe even one with different formats?
> 
> +1 for all values, +0 for multiple formats -- the flag should be tested, but
we
> don't need to test its behaviour exhaustively.

The all values is covered below in TestAllValues.

I deliberately kept these tests separate, as indicated by TestSingleValue :)

[thumper, 2013-04-11 00:20:39]

*** Submitted:

Add a get-environment command.

Be able to get environment values from state.  You can ask for either a single
value, or get the whole lot formatted how you like (well within reason).

set-environment coming next.

R=dfc, dimitern, fwereade
CC=
https://codereview.appspot.com/8597045