state: implement the single FindEntity method

state: implement the single FindEntity method

We rely on clients to do the type conversions
themselves if they want, rather than trying to
second-guess every possible interface that
a they might want to use.

Create a new state/interface.go file to hold interfaces
that entities can conform to.

https://code.launchpad.net/~rogpeppe/juju-core/363-agent-entity/+merge/178825

Requires: https://code.launchpad.net/~rogpeppe/juju-core/362-names-parse-tag/+merge/178619

(do not edit description out of merge proposal)

[rog, 2013-08-06 17:59:26]

Please take a look.

[dimitern, 2013-08-07 09:13:09]

Looks really good, but I'm concerned about some test cases in apiserver/common
being removed. Also needs merging trunk changes in.

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/life...
File state/apiserver/common/life_test.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/life...
state/apiserver/common/life_test.go:55: "x3": {err: fmt.Errorf("x3 error")},
Please, do not remove this case.

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
File state/apiserver/common/remove.go (right):

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
state/apiserver/common/remove.go:28: func (r *Remover) removeEntity(entityp
params.Entity) error {
s/entityp params.Entity/tag string/

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
File state/apiserver/common/remove_test.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
state/apiserver/common/remove_test.go:67: "x5": {life: state.Dead, err:
fmt.Errorf("x5 error")},
Don't remove this as well please.

https://codereview.appspot.com/12551043/diff/3001/state/state.go
File state/state.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/state.go#oldcode467
state/state.go:467: // Tagger represents an entity with a tag.
Yay!

https://codereview.appspot.com/12551043/diff/3001/state/state_test.go
File state/state_test.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/state_test.go#oldcode...
state/state_test.go:1538: // TODO(fwereade): when lp:1199352 (relation lacks
Tag) is fixed, check
Please, graft this TODO comment somewhere in the new code.

[rog, 2013-08-07 12:20:43]

Please take a look.

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/life...
File state/apiserver/common/life_test.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/life...
state/apiserver/common/life_test.go:55: "x3": {err: fmt.Errorf("x3 error")},
On 2013/08/07 09:13:09, dimitern wrote:
> Please, do not remove this case.

Done.

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
File state/apiserver/common/remove.go (right):

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
state/apiserver/common/remove.go:28: func (r *Remover) removeEntity(entityp
params.Entity) error {
On 2013/08/07 09:13:09, dimitern wrote:
> s/entityp params.Entity/tag string/

Done.

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
File state/apiserver/common/remove_test.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/apiserver/common/remo...
state/apiserver/common/remove_test.go:67: "x5": {life: state.Dead, err:
fmt.Errorf("x5 error")},
On 2013/08/07 09:13:09, dimitern wrote:
> Don't remove this as well please.

Done.

https://codereview.appspot.com/12551043/diff/3001/state/state_test.go
File state/state_test.go (left):

https://codereview.appspot.com/12551043/diff/3001/state/state_test.go#oldcode...
state/state_test.go:1538: // TODO(fwereade): when lp:1199352 (relation lacks
Tag) is fixed, check
On 2013/08/07 09:13:09, dimitern wrote:
> Please, graft this TODO comment somewhere in the new code.

Done.

[dimitern, 2013-08-07 12:56:26]

LGTM with that TODO reinstated.

https://codereview.appspot.com/12551043/diff/17010/state/apiserver/common/ens...
File state/apiserver/common/ensuredead.go (left):

https://codereview.appspot.com/12551043/diff/17010/state/apiserver/common/ens...
state/apiserver/common/ensuredead.go:18: type DeadEnsurerer interface {
I'm gonna miss that :)

https://codereview.appspot.com/12551043/diff/17010/state/apiserver/common/ens...
File state/apiserver/common/ensuredead.go (right):

https://codereview.appspot.com/12551043/diff/17010/state/apiserver/common/ens...
state/apiserver/common/ensuredead.go:35: return NotSupportedError(tag, "ensuring
death")
so grim.. :) but I have not better suggestion.

https://codereview.appspot.com/12551043/diff/17010/state/state_test.go
File state/state_test.go (left):

https://codereview.appspot.com/12551043/diff/17010/state/state_test.go#oldcod...
state/state_test.go:1538: // TODO(fwereade): when lp:1199352 (relation lacks
Tag) is fixed, check
Where is this TODO?

[rog, 2013-08-07 13:02:34]

Please take a look.

[rog, 2013-08-07 14:24:41]

Please take a look.

[gz, 2013-08-07 14:54:42]

LGTM. Kills the -erers which is grand, and the extra cast/check
error/NotImplementedError everywhere is offset by the test fakes becoming much
simpler.

https://codereview.appspot.com/12551043/diff/26001/state/apiserver/common/err...
File state/apiserver/common/errors.go (right):

https://codereview.appspot.com/12551043/diff/26001/state/apiserver/common/err...
state/apiserver/common/errors.go:24: func NotSupportedError(entity, operation
string) error {
I wonder if this wants a less generic name? I'm not sure the package prefix is
enough to say this is about entities not supporting operations.

[rog, 2013-08-07 15:00:48]

https://codereview.appspot.com/12551043/diff/26001/state/apiserver/common/err...
File state/apiserver/common/errors.go (right):

https://codereview.appspot.com/12551043/diff/26001/state/apiserver/common/err...
state/apiserver/common/errors.go:24: func NotSupportedError(entity, operation
string) error {
On 2013/08/07 14:54:42, gz wrote:
> I wonder if this wants a less generic name? I'm not sure the package prefix is
> enough to say this is about entities not supporting operations.

A reasonable point. I'll change it to use a better name,
if I can think of one.