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

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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}}},
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.

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())
Should do the access check first to avoid an unnecessary mongo trip.

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

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

« Back to merge proposal