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

Please take a look.

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

https://codereview.appspot.com/80230043/diff/1/state/api/client.go#newcode141
state/api/client.go:141: Entities: []params.EntityStatus{{Tag: machine,
Data: params.StatusData{"transient": true}}},
On 2014/03/26 02:54:08, axw wrote:
> This is a bit too leaky. The client API should be taking (machine)
tags and
> setting the transient=true status on the server side, otherwise we're
stuck with
> this implementation detail forever.

I didn't want to encode the ResolveProvisioningError API as gospel in
case we decided not to go with it long term, and thought a generic
UpdateMachineStatus API would be generically useful. The implementation
detail is behind a client fasçade. But it is leaky I agree in terms of
being stuck supporting this client version. I'll change it.

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

https://codereview.appspot.com/80230043/diff/1/state/apiserver/provisioner/provisioner.go#newcode239
state/apiserver/provisioner/provisioner.go:239: canAccess :=
canAccessFunc(machine.Tag())
On 2014/03/26 02:54:08, axw wrote:
> Should do the access check first to avoid an unnecessary mongo trip.

Done.

https://codereview.appspot.com/80230043/diff/1/state/apiserver/provisioner/provisioner.go#newcode243
state/apiserver/provisioner/provisioner.go:243: if err == nil ||
!canAccess {
On 2014/03/26 02:54:08, axw wrote:
> This is confusing, but I think you just want
> if err == nil && !canAccess {
> err = common.ErrPerm
> }
> ... right?

Changed to due above refactoring

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