state/apiserver: more ops on ResourceRegistry

state/apiserver: more ops on ResourceRegistry

For the upcoming API restructuring, packages external
of the apiserver package will need to retrieve and
stop resources. This adds the necessary operations to
ResourceRegistry and changes various places to use
them in preparation for factoring them out of apiserver
itself.

https://code.launchpad.net/~rogpeppe/juju-core/330-apiserver-resource-get/+merge/171364

(do not edit description out of merge proposal)

[rog, 2013-06-25 16:40:06]

Please take a look.

[fwereade, 2013-06-26 14:47:44]

LGTM modulo the tomb question. It'd clearly be a behaviour change, and so not
for this CL, but it feels a bit like it might be a bug waiting to happen?

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/watcher.go
File state/apiserver/watcher.go (right):

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/watcher.go#...
state/apiserver/watcher.go:40: if _, ok := <-w.watcher.Changes(); ok {
Is there some sort of tomb we should be selecting on here?

[dimitern, 2013-06-26 14:54:00]

LGTM modulo the two comments below.

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/machine/mac...
File state/apiserver/machine/machiner_test.go (right):

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/machine/mac...
state/apiserver/machine/machiner_test.go:36: panic("unimplemented")
why panic? return registry[id] instead

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/machine/mac...
state/apiserver/machine/machiner_test.go:40: panic("unimplemented")
ditto - return registry[id].Stop()? (if id is in registry)

[rog, 2013-06-26 15:01:09]

Please take a look.

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/machine/mac...
File state/apiserver/machine/machiner_test.go (right):

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/machine/mac...
state/apiserver/machine/machiner_test.go:36: panic("unimplemented")
On 2013/06/26 14:54:00, dimitern wrote:
> why panic? return registry[id] instead

i'm panicing because i don't expect these methods to be called in these tests;
no point in writing code that's never used. with john's upcoming shared
fakeResourceRegistry, i'm sure these will be fleshed out.

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/watcher.go
File state/apiserver/watcher.go (right):

https://codereview.appspot.com/10554043/diff/3001/state/apiserver/watcher.go#...
state/apiserver/watcher.go:40: if _, ok := <-w.watcher.Changes(); ok {
On 2013/06/26 14:47:45, fwereade wrote:
> Is there some sort of tomb we should be selecting on here?

no, not AFAICS - the watcher itself has a tomb, and when it dies, it closes the
channel.