consistency of usage of state.Relations

consistency of usage of state.Relations

ie:

(*State) AddRelation(...state.RelationEndpoint) (*Relation, error)
(*State) Relation(...state.RelationEndpoint) (*Relation, error)
(*State) RemoveRelation(*Relation) error

...and, also, RemoveService now:

* removes relations
* checks errors

https://code.launchpad.net/~fwereade/juju-core/state-relation-consistency/+merge/116240

Requires: https://code.launchpad.net/~fwereade/juju-core/relation-unit-methods/+merge/115924

(do not edit description out of merge proposal)

[fwereade, 2012-07-23 10:38:55]

Please take a look.

[niemeyer, 2012-07-23 10:58:45]

Nice stuff. Just needs some trivials sorted:

https://codereview.appspot.com/6351123/diff/1/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/6351123/diff/1/state/service_test.go#newcode430
state/service_test.go:430: rel1, err := s.State.AddRelation(mysqlep, wp1ep)
Why rel1?

https://codereview.appspot.com/6351123/diff/1/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6351123/diff/1/state/state.go#newcode385
state/state.go:385: func (s *State) Relation(endpoints ...RelationEndpoint)
(*Relation, error) {
This needs to consider error messages more carefully.

https://codereview.appspot.com/6351123/diff/1/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/6351123/diff/1/state/state_test.go#newcode339
state/state_test.go:339: c.Assert(err, ErrorMatches, `can't remove service
"wordpress": can't get relations for service "wordpress": service with key
"service-0000000000" not found`)
That's a message we don't want to show the user. This should talk about stuff
the user can correlate to.

[fwereade, 2012-07-23 14:18:03]

Please take a look.

https://codereview.appspot.com/6351123/diff/1/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/6351123/diff/1/state/service_test.go#newcode430
state/service_test.go:430: rel1, err := s.State.AddRelation(mysqlep, wp1ep)
On 2012/07/23 10:58:45, niemeyer wrote:
> Why rel1?

Um. Done.

https://codereview.appspot.com/6351123/diff/1/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6351123/diff/1/state/state.go#newcode385
state/state.go:385: func (s *State) Relation(endpoints ...RelationEndpoint)
(*Relation, error) {
On 2012/07/23 10:58:45, niemeyer wrote:
> This needs to consider error messages more carefully.

Done.

https://codereview.appspot.com/6351123/diff/1/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/6351123/diff/1/state/state_test.go#newcode339
state/state_test.go:339: c.Assert(err, ErrorMatches, `can't remove service
"wordpress": can't get relations for service "wordpress": service with key
"service-0000000000" not found`)
On 2012/07/23 10:58:45, niemeyer wrote:
> That's a message we don't want to show the user. This should talk about stuff
> the user can correlate to.

Done.

[niemeyer, 2012-07-23 17:16:28]

https://codereview.appspot.com/6351123/diff/6001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6351123/diff/6001/state/state.go#newcode416
state/state.go:416: return stateChanged
It'd be nicer to return something like "relation foo bar not found", where foo
bar is the user-name we give to the relation. Do we have that at hand here just
with the memory state of r?

[fwereade, 2012-07-23 18:36:08]

Please take a look.

https://codereview.appspot.com/6351123/diff/6001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6351123/diff/6001/state/state.go#newcode416
state/state.go:416: return stateChanged
On 2012/07/23 17:16:28, niemeyer wrote:
> It'd be nicer to return something like "relation foo bar not found", where foo
> bar is the user-name we give to the relation. Do we have that at hand here
just
> with the memory state of r?

Done.

[fwereade, 2012-07-23 18:38:59]

*** Submitted:

consistency of usage of state.Relations

ie:

(*State) AddRelation(...state.RelationEndpoint) (*Relation, error)
(*State) Relation(...state.RelationEndpoint) (*Relation, error)
(*State) RemoveRelation(*Relation) error

...and, also, RemoveService now:

* removes relations
* checks errors

R=niemeyer
CC=
https://codereview.appspot.com/6351123