Code review comment for lp:~wallyworld/juju-core/machineswithtransienterrors-api

Please take a look.

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go
File state/api/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go#newcode139
state/api/provisioner/provisioner_test.go:139: c.Assert(info[0].Error,
gc.ErrorMatches, "permission denied")
On 2014/03/26 04:57:01, axw wrote:
> This seems like a weird situation to be in. If there's a machine that
the caller
> is not allowed to read, I think it should just be excluded.

Done.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go
File state/apiserver/provisioner/provisioner.go (right):

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode244
state/apiserver/provisioner/provisioner.go:244: if canAccess && err ==
nil {
On 2014/03/26 04:57:01, axw wrote:
> err == nil implies canAccess (otherwise it'd be ErrPerm), so just "if
err ==
> nil"

Done.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode249
state/apiserver/provisioner/provisioner.go:249: if transient, ok :=
result.Data["transient"]; !ok || !transient.(bool) {
On 2014/03/26 04:57:01, axw wrote:
> There's a potential type assertion panic here, if something sets
transient to
> something other than a bool. Not sure if we want to be bothered
working around
> that.

I thought about it but forgot. Fixed.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode254
state/apiserver/provisioner/provisioner.go:254: if err == nil {
On 2014/03/26 04:57:01, axw wrote:
> It doesn't seem right to me that MachinesWithTransientErrors should be
returning
> errors for individual machines. Machine that are inaccessible simply
shouldn't
> show up, I think. The caller isn't asking about them specifically.

Hmmmm. I think that's a valid point. I've changed it.

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