Code review comment for lp:~themue/pyjuju/go-state-remove-relation

https://codereview.appspot.com/6250076/diff/6032/state/relation.go
File state/relation.go (right):

https://codereview.appspot.com/6250076/diff/6032/state/relation.go#newcode84
state/relation.go:84: func (r *ServiceRelation) Relation() *Relation {
This needs a test.

https://codereview.appspot.com/6250076/diff/6032/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/6250076/diff/6032/state/state_test.go#newcode1136
state/state_test.go:1136: func (s *StateSuite)
TestRemoveRServiceRelation(c *C) {
Please drop this test. RemoveRelation is tested above. We don't have to
test that RemoveRelation works with the result of
ServiceRelation.Relation. We have to test that ServiceRelation.Relation,
by itself, works.

https://codereview.appspot.com/6250076/diff/6032/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6250076/diff/6032/state/topology.go#newcode403
state/topology.go:403: return nil, fmt.Errorf("relation does not exist")
Please revert this change. The goal isn't to prevent this from showing
the key. The goal is to make sure that the high level layer presents a
*good* high-level error.

Going through the state code I see that errors continue to be all over
the place. We've talked about this before, and didn't really do it. Now
may be the right time to stop and go back to fix our debt in the error
management of the state code.

We can go ahead and integrate this, but please do revert this change so
that it continues to show the proper key in the low-level interface
error.

In the high-level interface, please add a TODO so we remember it's
broken.

https://codereview.appspot.com/6250076/