Modified support for working with relations

This proposal pulls together a number of changes, backed by use cases
from bug reports, for modifying Juju's current relation support as
follows:

 * Enable working with relation ids. This includes adding a new
   environment variable `$JUJU_RELATION_ID` for relation hooks.

 * Enable `relation-get`, `relation-set`, `relation-list` to work in
   any hook on arbitrary relations, as specified by a relation name or
   unambiguously by relation id.

 * Add a new relation hook command, `relation-info`, for listing the
   relation ids associated with a service.

 * Fix a bug in `juju status` status for the scenario that a service
   has multiple established relations to consuming clients, each using
   the same relation name from the provider's perspective. This fix
   does introduce a backwards breaking change.

https://code.launchpad.net/~jimbaker/juju/enhanced-relation-spec/+merge/95268

(do not edit description out of merge proposal)

[Jim Baker, 2012-02-29 21:41:25]

Please take a look.

[hazmat, 2012-03-01 05:42:08]

Needs Fixing + Needs Information.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
File source/drafts/enhanced-relations.rst (right):

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:15: 
I'd switch this to.. Supports unambiguously referencing relations by a relation
identifier.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:20: 
Not sure where this came from, i don't see it referenced from any bugs. Your
talking about arbitrary hook execution driven by a user instead of by a charm,
which is not only dangerous, but imo an anti-use case.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:24: 
The issue isn't complex usage. Its any interface provider having more than one
client. ie. mysql with two relations. 

i seeing the words 'enhance', 'important', and 'flexible' reused alot, i think
most of them are pointless.

i'd switch to this bulletpoint to "Enable status to report interface providers
that have multiple relationships."

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:44: 
There's no need to separate this block, i'd rename the original block to
Relation Identifiers.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:68: 
This sort of misses the point of the identifers. Ie. why are they needed, why
isn't relation name adequate. Its only the server endpoint for which this is
needed, which the document does not describe, and its only the context of those
relations that identifiers other than name are needed.

I'd drop all the other discussion of forms outside of  interface name with
rel-id, which is the only relevant one. ie. the only one preserving semantic
context and uniqueness.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:75: 
Both these sentences can be dropped.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:85: 
where rel name is unique, i think it should be used for rel-id. ie. i'm
upgrading wordpress, i know exactly what rel i need to touch to modify the db.
using identifiers requires analysis of runtime state for an id which is unesc.
when the charm def already has a unique name.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:99: 
-r rel_name_or_id suffices

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:248: 
This ml thread doesn't suggest that.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:315: 
This whole block should be ditched.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:357: relation-0:
This should never be displayed.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:368: role: consumes
This role does not exit.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:400: 
We've stated a bug, but your talking about implementing a bunch of stuff
completely unrelated in this expanded relation status output.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:466: 
There are two different scenarios being spec'd one of which is not needed. ie.
client side juju do.. is not needed, that's arbitrary hook execution by a user.
being able to invoke relation hook cli api from non relation hooks is needed.

[hazmat, 2012-03-01 19:48:08]

more comments from joint discussion.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
File source/drafts/enhanced-relations.rst (right):

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:16: * Enable `relation-get`,
`relation-set`, `relation-list` to work in
+ a yet to be named command for listing all the relations, in preference to
overriding relation-list.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:25: 
We don't need to expand status output, we just need to clean it up to be
correct.

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:144: 
relation-list expanded format should be a separate command


The syntax should be more along the lines of 

relation-list [--format FORMAT] RELATION_NAME

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:165: 
These are unesc (interface, role, scope)

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:351: state: started
So we want this to be.. 

 relations: 
   - {name: db, service: wordpress, state: up}

or

 relations: 
    db: 
      - {service: wordpress, state: up}
      - {service: mediawiki, state: up}

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:449: 
we're going to skip out of band hook execution

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:492: 
This should be yanked. This sort of thing belongs in juju-jitsu

http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations...
source/drafts/enhanced-relations.rst:559: 
This isn't needed. relation-id is still used for lookup.

[Jim Baker, 2012-03-07 05:43:51]

Please take a look.

[niemeyer, 2012-03-15 18:58:38]

Discussed on IRC:

<niemeyer> jimbaker: Sure, will review it onw
<niemeyer> now
<jimbaker> niemeyer, thanks
<niemeyer> jimbaker: This looks a bit like a bag-of-changes spec.. these changes
being described in the summary look quite unrelated to each other
<jimbaker> niemeyer, ok
<niemeyer> jimbaker: There's a pretty clear hint of that in the topic of the
spec actually: "Modified support for working with relations"
* fwereade__ has quit (Ping timeout: 260 seconds)
<jimbaker> niemeyer, you are absolutely right about that title, it was very
intentional
<niemeyer> jimbaker: As in, intentionally vague? Why?
<jimbaker> niemeyer, this is what i was asked to work on
<jimbaker> nonetheless, there's only this common theme
<jimbaker> that it is working on a number of features about relations
<niemeyer> jimbaker: on something intentionally vague? Sorry, I'm not following
what you mean..
<niemeyer> jimbaker: Yeah, but you being asked to work on a number of things is
something unrelated to having a spec that describes a number of unrelated things
<niemeyer> jimbaker: We can reach a different agreement on each one of those
things
<niemeyer> jimbaker: and this single spec will be blocked while we can't reach
an agreement on all of them
<jimbaker> it's not meant to be intentionally vague, but it does cover a range
of useful topics
<jimbaker> if you want to consider it a set of separate proposals, that probably
works
<jimbaker> and we can work on each one on in turn
<niemeyer> jimbaker: I'll go over it because I guess that's the shortest path to
action by now, but it's better to have more clearly defined specs and goals. You
could have some of those bits already done by now, for example, if you had
started with a single one of them
<jimbaker> niemeyer, ok
<niemeyer> jimbaker: Actually, that's not true..
<niemeyer> jimbaker: We can probably get approval on some of those aspects very
quickly
<niemeyer> jimbaker: and you might start working on them right away
<jimbaker> niemeyer, that's good to hear
<niemeyer> jimbaker: Can you please do something very quick: take those
independent concerns apart and put them in separate branches, and repush for
review
<jimbaker> niemeyer, ok
* Leseb has quit (Quit: Leseb)
<niemeyer> jimbaker: I'll review all of them in turn right away
<niemeyer> jimbaker: But I expect we'll be able to get some of them in quickly,
while others will require more back and forth
<niemeyer> jimbaker: You can work on the initial ones first, while we reach
agreement in the rest
<jimbaker> niemeyer, sounds good
<niemeyer> jimbaker: Thanks, and sorry for the trouble