Hi Jim. This is a pretty big merge, so perhaps next time use lbox propose -cr so we can review w/ rietveld?
Anyway, I think it all LGTM .. one bit of feedback:
99 - raise RelationStateNotFound()
100 + parts = relation_ident.split(":")
101 + if len(parts) != 2 or not parts[1].isdigit():
102 + raise InvalidRelationIdentity(relation_ident)
103 + else:
104 + raise RelationStateNotFound()
I don't really understand checking the format at this point.. we already know its not found, and thats what a user trying to debug their charm would want to know. The format problem would likely be *more* confusing. So I think this check, and the extra test for it, can just be dropped.
Hi Jim. This is a pretty big merge, so perhaps next time use lbox propose -cr so we can review w/ rietveld?
Anyway, I think it all LGTM .. one bit of feedback:
99 - raise RelationStateNo tFound( ) ident.split( ":") Identity( relation_ ident) tFound( )
100 + parts = relation_
101 + if len(parts) != 2 or not parts[1].isdigit():
102 + raise InvalidRelation
103 + else:
104 + raise RelationStateNo
I don't really understand checking the format at this point.. we already know its not found, and thats what a user trying to debug their charm would want to know. The format problem would likely be *more* confusing. So I think this check, and the extra test for it, can just be dropped.