Hook up the debug-log command line to the API.

Hook up the debug-log command line to the API.

Have 'juju debug-log' try to use the API first. If that
fails, it falls back to tailing the log over ssh.

https://code.launchpad.net/~thumper/juju-core/debug-log-api/+merge/215323

(do not edit description out of merge proposal)

[thumper, 2014-04-11 00:27:56]

Please take a look.

[wallyworld, 2014-04-11 00:54:43]

LGTM, not sure about the need for a different "api not supported" mechanism
though.

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

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode46
cmd/args.go:46: // f.Var(cmd.NewAppendStringsValue(defaultValue, &someMember),
"name", "help")
The above comment is wrong

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode59
cmd/args.go:59: return fmt.Sprint(*v)
Could we do:

return strings.Join(*v, ",")

to be consistent with StringsValue

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

https://codereview.appspot.com/85570044/diff/1/cmd/args_test.go#newcode161
cmd/args_test.go:161: message:       "no value set by args",
I don't understand the message. Shouldn't it be "value set by args"?

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

https://codereview.appspot.com/85570044/diff/1/cmd/juju/debuglog.go#newcode74
cmd/juju/debuglog.go:74: c.level, loggo.TRACE, loggo.DEBUG, loggo.INFO,
loggo.WARNING, loggo.ERROR)
Would be nice if there were a loggo helper ValidLevels()

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

https://codereview.appspot.com/85570044/diff/1/state/api/client.go#newcode773
state/api/client.go:773: return nil,
errors.NewNotSupportedError("WatchDebugLog")
Till now, CodeNotImplemented has been used to flag an incompatible API call. I'm
not sure of the value of introducing something new here, instead of just going
with what's already been done.

[thumper, 2014-04-11 01:05:35]

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

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode46
cmd/args.go:46: // f.Var(cmd.NewAppendStringsValue(defaultValue, &someMember),
"name", "help")
On 2014/04/11 00:54:43, wallyworld wrote:
> The above comment is wrong

Done.

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode59
cmd/args.go:59: return fmt.Sprint(*v)
On 2014/04/11 00:54:43, wallyworld wrote:
> Could we do:
> 
> return strings.Join(*v, ",")
> 
> to be consistent with StringsValue

Done.

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

https://codereview.appspot.com/85570044/diff/1/cmd/args_test.go#newcode161
cmd/args_test.go:161: message:       "no value set by args",
On 2014/04/11 00:54:43, wallyworld wrote:
> I don't understand the message. Shouldn't it be "value set by args"?

yes

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

https://codereview.appspot.com/85570044/diff/1/state/api/client.go#newcode773
state/api/client.go:773: return nil,
errors.NewNotSupportedError("WatchDebugLog")
On 2014/04/11 00:54:43, wallyworld wrote:
> Till now, CodeNotImplemented has been used to flag an incompatible API call.
I'm
> not sure of the value of introducing something new here, instead of just going
> with what's already been done.

Talked with Roger about this last night and we agreed to change it to this.