Merge lp://staging/~waigani/juju-core/user-detail into lp://staging/~go-bot/juju-core/trunk

Reviewers: mp+221314_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: user detail

Add a new subcommand to `juju user` which
prints out the details of the current
user.

Add a function on the apiserver,
CurrentUserInfo. Add a function on the api
client which calls the server side function
print out the result via the cmd.Output
formatter, allowing json and yaml outputs.

https://code.launchpad.net/~waigani/juju-core/user-detail/+merge/221314

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/98700043/

Affected files (+178, -0 lines):
   A [revision details]
   A cmd/juju/user_detail.go
   A cmd/juju/user_detail_test.go
   M state/api/client.go
   M state/api/params/internal.go
   M state/api/usermanager/client.go
   M state/apiserver/client/client.go

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

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode20
cmd/juju/user_detail.go:20: user-name: admin
How about using examples that don't use admin?

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode28
cmd/juju/user_detail.go:28: user-name: admin
I think we should have a big TODO here somewhere that says: as we add
information to the user in the database, we will show it here.

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode47
cmd/juju/user_detail.go:47: c.out.AddFlags(f, "smart",
cmd.DefaultFormatters)
I think we should have the default formatter not output "smart" yaml,
but instead be a simple output that just says the name:

$ juju user detail
admin

$ juju user detail --format json
{"user-name": "admin"}

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode60
cmd/juju/user_detail.go:60: findResult, err := client.CurrentUserInfo()
This isn't handling the case where we have a user specified on the
command line.

We should be saying:
   client.UserInfo(username)

https://codereview.appspot.com/98700043/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/98700043/diff/1/state/api/client.go#newcode861
state/api/client.go:861: func (c *Client) CurrentUserInfo() (result
params.CurrentUserInfoResults, err error) {
As mentioned elsewhere, this is the wrong name...

https://codereview.appspot.com/98700043/

This does not LGTM as it stands. Along with the issues with the API
itself, there are no tests for anything api-related. I would strongly
suggest writing a CL that *just* adds the API, and to worry about the
CLI once that's solid.

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

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode14
cmd/juju/user_detail.go:14: const userDetailCommandDoc = `
FWIW, it might be worthwhile separating the intended future docs from
the in-command docs -- as it is, while it doesn't yet handle username,
this is just inaccurate.

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode28
cmd/juju/user_detail.go:28: user-name: admin
On 2014/05/29 05:15:01, thumper wrote:
> I think we should have a big TODO here somewhere that says: as we add
> information to the user in the database, we will show it here.

This does feed into how we want to write the smart formatter. Just
outputting "admin" (or "thumper") is going to become more unhelpful as
we get more fields, especially as those fields' meaning becomes harder
to infer from content alone.

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode47
cmd/juju/user_detail.go:47: c.out.AddFlags(f, "smart",
cmd.DefaultFormatters)
On 2014/05/29 05:15:01, thumper wrote:
> I think we should have the default formatter not output "smart" yaml,
but
> instead be a simple output that just says the name:

> $ juju user detail
> admin

> $ juju user detail --format json
> {"user-name": "admin"}

FWIW, I don't think "smart" format has to be entirely consistent across
commands. My conception of it has always been "generate nicely
human-readable output (with an option on easy handling in bash)" -- if
you want easily-parseable output, that's what json/yaml are for.

Writing an output-type-specific "smart" formatter for each command seems
like a pretty low-friction way to go, tbh; and indeed *sometimes* the
default smart formatter will be perfectly adequate (eg, it would be
pretty nice for just printing out lists of api endpoints one-per-line)

https://codereview.appspot.com/98700043/diff/1/cmd/juju/user_detail.go#newcode60
cmd/juju/user_detail.go:60: findResult, err := client.CurrentUserInfo()
On 2014/05/29 05:15:01, thumper wrote:
> This isn't handling the case where we have a user specified on the
command line.

> We should be saying:
> client.UserInfo(username)

...and we should be figuring out the current user to begin with, and
asking for UserInfo by name in all cases anyway.

You're writing bulk API calls, right? I don't see how CurrentUserInfo is
amenable to that... if you want a magic value that means "the logged-in
user" I could *maybe* live with that, but really, whatever makes the
connection ought to know on whose behalf it's made the connection in the
first place, and should ask explicitly for that user's info.

(maybe you handle this elsewhere, I haven't read the whole CL yet, but
I'm not even convinced that the cost/benefit of CurrentUserInfo really
helps much even as a pure client-side convenience method)

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