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

<niemeyer> TheMue: I see you're renaming from zkRelation to
topoRelation.. are tests broken at the moment without that?
<TheMue> niemeyer: Without renaming it has been broken.
<TheMue> niemeyer: Yes.
<niemeyer> TheMue: Ok :(
<niemeyer> TheMue: Trunk is being broken way too often :(
<niemeyer> 382 t.RemoveRelation(relation.key)
<niemeyer> 383 return nil
<niemeyer> TheMue: Is RemoveRelation returning no errorr?
<TheMue> niemeyer: No, it's only removing inside a map. Today it's also
not checked at that point if it exists in that dir.
<niemeyer> TheMue: OK
<niemeyer> / Relation returns the relation with key from the topology.
<niemeyer> func (t *topology) Relation(key string) (*topoRelation,
error) {
<niemeyer> if t.topology.Relations == nil ||
t.topology.Relations[key] == nil {
<niemeyer> return nil, fmt.Errorf("relation %q does not
exist", key)
<niemeyer> TheMue: We don't want the user to be getting this error
message
<TheMue> niemeyer: Oh, please wait.Just seen an assert_relation there.
Will add it too.
<niemeyer> TheMue: The relation key is completely uninteresting as far
as a user is concerned
<niemeyer> TheMue: The state method should verify the state in those
cases, and error early with a sensible message
<TheMue> niemeyer: OK
<niemeyer> TheMue: If we get an Relation from relation we should display
prefixed with something saying that an unexpected action happened while
doing whatever
<niemeyer> TheMue: In which case the relation key might go out, but we
have no idea about why it was failing since we've checked it early
<niemeyer> TheMue: So it's debugging rather than something we'd like the
user to be commonly looking at
<TheMue> niemeyer: IC. Do you note it in the review?
<niemeyer> TheMue: I'll copy & paste what I just said

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