state: Improved verification of relation endpoints.

state: Improved verification of relation endpoints.

AddRelation() consists of several writings to ZooKeeper. Now
the whole verification is done before the writings start.

https://code.launchpad.net/~themue/juju-core/go-state-relation-endpoint-verification/+merge/109171

(do not edit description out of merge proposal)

[TheMue, 2012-06-07 15:35:58]

Please take a look.

[rog, 2012-06-07 16:17:45]

LGTM, although i'm sure i don't understand all the implications.

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

https://codereview.appspot.com/6305067/diff/1/state/relation.go#newcode63
state/relation.go:63: return fmt.Errorf("endpoint has invalid role: %q",
e.RelationRole)
s/://

generally we use ":" for indicating sub-errors, not for information within a
particular error.

[fwereade, 2012-06-08 17:13:02]

I'm not really convinced this helps us much. The only sane way to sanely
construct a RelationEndpoint is from a service+charm in the first place... are
we worried that we'll get bad data from those, or that ill-disciplined
programmers will be constructing them at random and flinging them at the wall to
see what sticks?

(I presume we're really worried about something else I haven't thought of.
Elaborate please?)

[fwereade, 2012-06-11 08:22:47]

I still doubt this will have great value in practice, but LGTM.

[niemeyer, 2012-06-13 01:27:36]

On 2012/06/11 08:22:47, fwereade wrote:
> I still doubt this will have great value in practice, but LGTM.

Sorry Frank, I have to agree with William, even though the suggestion to prevent
this from happening was mine in the first place.

My misguided suggestion was based on the fact I had wrong memories regarding the
way we handle "deploy serviceA:foo serviceB:bar" commands. We don't create the
relation endpoint directly, but rather call a method to resolve what we call the
*descriptor* "<service>[:<relname>]". This is the place that must have that
validation to prevent errors from the user from leaking through.

I apologize for wasting your time on this.