worker: trivial machiner

worker: trivial machiner

Sets Dying machines to Dead. That's it.

https://code.launchpad.net/~fwereade/juju-core/machiner-lifecycle/+merge/142321

(do not edit description out of merge proposal)

[fwereade, 2013-01-08 14:28:42]

Please take a look.

[TheMue, 2013-01-08 14:33:01]

LGTM

[dimitern, 2013-01-08 14:35:53]

On 2013/01/08 14:28:42, fwereade wrote:
> Please take a look.

LGTM

[rog, 2013-01-08 16:22:03]

not too keen on the name, but i can't think of anything else.
looks good, with a couple of suggestions for simplification below.

https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go
File worker/machiner/machiner.go (right):

https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go#new...
worker/machiner/machiner.go:21: func NewMachiner(st *state.State, id string)
*Machiner {
since jujud has already fetched the Machine, why not pass it into NewMachiner?
then you can lose the st argument (i think) and also the code that fetches the
machine from the state and its error checking.

https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go#new...
worker/machiner/machiner.go:62: switch life := m.Life(); life {
slightly simpler alternative, perhaps?

if m.Life() != state.Alive {
    if err := m.EnsureDead(); err != nil {
        return err
    }
    return worker.ErrDead
}

[fwereade, 2013-01-08 23:22:54]

Given 2 LGTMs and justification re NewMachiner awkwardness, submitting. 

On 2013/01/08 16:22:03, rog wrote:
> not too keen on the name, but i can't think of anything else.
> looks good, with a couple of suggestions for simplification below.
> 
> https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go
> File worker/machiner/machiner.go (right):
> 
>
https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go#new...
> worker/machiner/machiner.go:21: func NewMachiner(st *state.State, id string)
> *Machiner {
> since jujud has already fetched the Machine, why not pass it into NewMachiner?
> then you can lose the st argument (i think) and also the code that fetches the
> machine from the state and its error checking.
> 
>
https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go#new...
> worker/machiner/machiner.go:62: switch life := m.Life(); life {
> slightly simpler alternative, perhaps?
> 
> if m.Life() != state.Alive {
>     if err := m.EnsureDead(); err != nil {
>         return err
>     }
>     return worker.ErrDead
> }

[fwereade, 2013-01-08 23:24:33]

*** Submitted:

worker: trivial machiner

Sets Dying machines to Dead. That's it.

R=TheMue, dimitern, rog
CC=
https://codereview.appspot.com/7069055

https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go
File worker/machiner/machiner.go (right):

https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go#new...
worker/machiner/machiner.go:21: func NewMachiner(st *state.State, id string)
*Machiner {
On 2013/01/08 16:22:03, rog wrote:
> since jujud has already fetched the Machine, why not pass it into NewMachiner?
> then you can lose the st argument (i think) and also the code that fetches the
> machine from the state and its error checking.

Because it's already been passed into NewUpgrader and it seems foolish to assume
that it is untainted. I'm +1 on adding Clone methods, as discussed, but it's
worth doing that in a separate CL because I think it will be useful in several
places.

https://codereview.appspot.com/7069055/diff/1/worker/machiner/machiner.go#new...
worker/machiner/machiner.go:62: switch life := m.Life(); life {
On 2013/01/08 16:22:03, rog wrote:
> slightly simpler alternative, perhaps?
> 
> if m.Life() != state.Alive {
>     if err := m.EnsureDead(); err != nil {
>         return err
>     }
>     return worker.ErrDead
> }

Done.