state/api: Client-side Machiner API facade

state/api: Client-side Machiner API facade

This adds client-side API Machiner facade
and a lightweight proxy MachinerMachine,
which implements the methods on state.Machine,
as needed by the machiner worker, but using
the facade internally for the actual calls.

Also introduced client-side tests.

Fixed and re-enabled a test I broke earlier
in r1247.

https://code.launchpad.net/~dimitern/juju-core/053-api-machiner-client-side/+merge/167278

(do not edit description out of merge proposal)

[dimitern, 2013-06-04 13:33:10]

Please take a look.

[dimitern, 2013-06-04 13:39:21]

Please take a look.

[mue, 2013-06-04 13:49:14]

Nice, LGTM.

[rog, 2013-06-04 16:26:28]

https://codereview.appspot.com/9686047/diff/3001/state/api/machiner.go
File state/api/machiner.go (right):

https://codereview.appspot.com/9686047/diff/3001/state/api/machiner.go#newcode45
state/api/machiner.go:45: life, err := m.machineLife(id)
what's the plan here when we start to store other attributes in the
MachinerMachine?

[dimitern, 2013-06-05 07:16:04]

https://codereview.appspot.com/9686047/diff/3001/state/api/machiner.go
File state/api/machiner.go (right):

https://codereview.appspot.com/9686047/diff/3001/state/api/machiner.go#newcode45
state/api/machiner.go:45: life, err := m.machineLife(id)
On 2013/06/04 16:26:28, rog wrote:
> what's the plan here when we start to store other attributes in the
> MachinerMachine?

We don't need other attributes for the machiner to work, as it is now.

[dimitern, 2013-06-05 08:08:15]

Please take a look.

[fwereade, 2013-06-05 09:28:08]

LGTM, thanks.

https://codereview.appspot.com/9686047/diff/10001/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/9686047/diff/10001/cmd/jujud/machine_test.go#n...
cmd/jujud/machine_test.go:285: defer st.Close()
I don't feel the following stuff is necessary. Why are we testing anything other
than "we can log in to the API" here?

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner.go
File state/api/machiner.go (right):

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner.go#newco...
state/api/machiner.go:17: // the Machiner facade.
Can we have a comment saying "this icky name is temporary, all this
machiner-specific stuff will get its own package soon"?

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner_test.go
File state/api/machiner_test.go (right):

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner_test.go#...
state/api/machiner_test.go:146: c.Assert(err, ErrorMatches, "machine 0 not
found")
This is a change I'm not wild about, but I guess not really a big deal in the
wider scheme of things. So ignore me :).

[dimitern, 2013-06-05 10:02:55]

*** Submitted:

state/api: Client-side Machiner API facade

This adds client-side API Machiner facade
and a lightweight proxy MachinerMachine,
which implements the methods on state.Machine,
as needed by the machiner worker, but using
the facade internally for the actual calls.

Also introduced client-side tests.

Fixed and re-enabled a test I broke earlier
in r1247.

R=mue, rog, fwereade
CC=
https://codereview.appspot.com/9686047

https://codereview.appspot.com/9686047/diff/10001/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/9686047/diff/10001/cmd/jujud/machine_test.go#n...
cmd/jujud/machine_test.go:285: defer st.Close()
On 2013/06/05 09:28:08, fwereade wrote:
> I don't feel the following stuff is necessary. Why are we testing anything
other
> than "we can log in to the API" here?

Good point. For that, just the call to st.Machiner returning no error should be
enough. Will change.

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner.go
File state/api/machiner.go (right):

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner.go#newco...
state/api/machiner.go:17: // the Machiner facade.
On 2013/06/05 09:28:08, fwereade wrote:
> Can we have a comment saying "this icky name is temporary, all this
> machiner-specific stuff will get its own package soon"?

Done.

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner_test.go
File state/api/machiner_test.go (right):

https://codereview.appspot.com/9686047/diff/10001/state/api/machiner_test.go#...
state/api/machiner_test.go:146: c.Assert(err, ErrorMatches, "machine 0 not
found")
On 2013/06/05 09:28:08, fwereade wrote:
> This is a change I'm not wild about, but I guess not really a big deal in the
> wider scheme of things. So ignore me :).

As discussed before, will change this behavior as needed.