state/apiserver: Implement Machiner.Watch

state/apiserver/machiner/machiner.go

 // Copyright 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package machiner
 
 import (
         "launchpad.net/juju-core/state"
         "launchpad.net/juju-core/state/api/params"
         "launchpad.net/juju-core/state/apiserver/common"
-        statewatcher "launchpad.net/juju-core/state/watcher"
 )
 
 // Machiner implements the API used by the machiner worker.
 type Machiner struct {
         st               *state.State
         resourceRegistry common.ResourceRegistry

[rog, 2013/06/07 13:04:37]

s/resourceRegistry/resources/ ?

[dimitern, 2013/06/07 13:56:38]

I started liking longer, more descriptive names :)

         auth             common.Authorizer
 }
 
 // New creates a new instance of the Machiner facade.
-func New(st *state.State, resourceRegistry common.ResourceRegistry, authorizer common.Authorizer) *Machiner {

[rog, 2013/06/07 13:04:37]

s/resourceRegistry/resources/ ?

-        return &Machiner{st, resourceRegistry, authorizer}
+func New(st *state.State, resourceRegistry common.ResourceRegistry, authorizer common.Authorizer) (*Machiner, error) {
+        if !authorizer.IsLoggedIn() {
+                return nil, common.ErrNotLoggedIn
+        }
+        if !authorizer.AuthMachineAgent() {
+                return nil, common.ErrPerm
+        }
+        return &Machiner{st, resourceRegistry, authorizer}, nil
 }
 
 // SetStatus sets the status of each given machine.
 func (m *Machiner) SetStatus(args params.MachinesSetStatus) (params.ErrorResults, error) {
         result := params.ErrorResults{
                 Errors: make([]*params.Error, len(args.Machines)),
         }
         if len(args.Machines) == 0 {
                 return result, nil
         }
         for i, arg := range args.Machines {
                 machine, err := m.st.Machine(arg.Id)
                 if err == nil {
                         // Allow only for the owner agent.
                         if !m.auth.AuthOwner(machine) {
                                 err = common.ErrPerm
                         } else {
                                 err = machine.SetStatus(arg.Status, arg.Info)
                         }
                 }
-                result.Errors[i] = common.ServerErrorToParams(err)
+                result.Errors[i] = common.ServerError(err)
         }
         return result, nil
 }
 
 // Watch starts an EntityWatcher for each given machine.
 func (m *Machiner) Watch(args params.Machines) (params.MachinesWatchResults, error) {
         result := params.MachinesWatchResults{
                 Results: make([]params.MachineWatchResult, len(args.Ids)),
         }
         if len(args.Ids) == 0 {
                 return result, nil
         }
         for i, id := range args.Ids {
                 machine, err := m.st.Machine(id)
                 if err == nil {
                         // Allow only for the owner agent.
                         if !m.auth.AuthOwner(machine) {
                                 err = common.ErrPerm
                         } else {
                                 watcher := machine.Watch()
-                                // To save an extra round-trip to call Next after Watch, we check
-                                // for initial changes.

[fwereade, 2013/06/06 17:04:38]

I *think* I see what's going on here... the client will automatically send out
the guaranteed initial event, right? I don't love it, honestly... it feels like
an unnecessary distinction between entity watchers and other watchers at the API
level -- some will send initial events and some won't -- and I'm not convinced
the saving is worth the mental load and loss of clarity.

If we're not currently constrained by a client-side watcher implementation,
let's at least start by being consistent with our general plans for watchers; we
can optimize it if and when it becomes an issue.

[dimitern, 2013/06/07 11:47:54]

Will remove this from here and the client-side implementation will change
accordingly.

[rog, 2013/06/07 13:04:37]

There should be no such distinction between watchers at the API level.
The idea is that *all* watchers send the initial data with the
first Watch request. The only thing that's special about the
entity watcher is that there is no actual data to include in the Watch
reply.

AFAIK all watchers in state send an initial event.

Please can we leave this as is?

-                                if _, ok := <-watcher.Changes(); !ok {
-                                        err = statewatcher.MustErr(watcher)
-                                } else {
-                                        result.Results[i].EntityWatcherId = m.resourceRegistry.Register(watcher)
-                                }
+                                result.Results[i].EntityWatcherId = m.resourceRegistry.Register(watcher)
                         }
                 }
-                result.Results[i].Error = common.ServerErrorToParams(err)
+                result.Results[i].Error = common.ServerError(err)
         }
         return result, nil
 }
 
 // Life returns the lifecycle state of each given machine.
 func (m *Machiner) Life(args params.Machines) (params.MachinesLifeResults, error) {
         result := params.MachinesLifeResults{
                 Machines: make([]params.MachineLifeResult, len(args.Ids)),
         }
         if len(args.Ids) == 0 {
                 return result, nil
         }
         for i, id := range args.Ids {
                 machine, err := m.st.Machine(id)
                 if err == nil {
                         // Allow only for the owner agent.
                         if !m.auth.AuthOwner(machine) {
                                 err = common.ErrPerm
                         } else {
                                 result.Machines[i].Life = params.Life(machine.Life().String())
                         }
                 }
-                result.Machines[i].Error = common.ServerErrorToParams(err)
+                result.Machines[i].Error = common.ServerError(err)
         }
         return result, nil
 }
 
 // EnsureDead changes the lifecycle of each given machine to Dead if
 // it's Alive or Dying. It does nothing otherwise.
 func (m *Machiner) EnsureDead(args params.Machines) (params.ErrorResults, error) {
         result := params.ErrorResults{
                 Errors: make([]*params.Error, len(args.Ids)),
         }
         if len(args.Ids) == 0 {
                 return result, nil
         }
         for i, id := range args.Ids {
                 machine, err := m.st.Machine(id)
                 if err == nil {
                         // Allow only for the owner agent.
                         if !m.auth.AuthOwner(machine) {
                                 err = common.ErrPerm
                         } else {
                                 err = machine.EnsureDead()
                         }
                 }
-                result.Errors[i] = common.ServerErrorToParams(err)
+                result.Errors[i] = common.ServerError(err)
         }
         return result, nil
 }