Code review comment for lp:~jimbaker/pyjuju/rel-mgmt-add-relation-simple

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

Of course. I'm referring to the error checking you introduced in the branch as
a side effect of point 1 above. E.g.:

55 + if (topology.relation_has_service(
56 + service_relation.internal_relation_id,
57 + service_relation.internal_service_id) or
58 + topology.relation_has_role(
59 + service_relation.internal_relation_id,
60 + service_relation.relation_role)):
61 + raise ServiceRelationAlreadyAssigned(
62 + topology.get_service_name(
63 + service_relation.internal_service_id),
64 + service_relation.relation_name,
65 + service_relation.relation_role)

[2]

> We should assert the endpoints share a relation_type. In addition we now
> must have either one or two endpoints passed in, since we will eliminate the
> relation_type (and vestigial assign_service).

Sounds good. It doesn't ever make sense to create a relation without services, and
most probably, as we discussed yesterday, it doesn't make sense to assign new
services to an existing relation either.

« Back to merge proposal