Add `relation-info` relation hook command.

Add `relation-info` relation hook command.



https://code.launchpad.net/~jimbaker/juju/relation-info-command-spec/+merge/97736

(do not edit description out of merge proposal)

[jimbaker, 2012-03-15 19:44:23]

Please take a look.

[jimbaker, 2012-03-19 18:46:21]

Please take a look.

[niemeyer, 2012-03-20 19:47:35]

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info.rst
File source/drafts/relation-info.rst (right):

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:7: relation-info [--interface INTERFACE] [-r
RELATION_NAME_OR_ID]
If this command enumerates relation ids, how can it accept a relation id as a
parameter? Also, I don't see how it can take a service name as a parameter
either, or why it takes an interface, or even a unit name.

If I understand what this command is doing, I'd imagine something much simpler
instead:

    relation-ids [<relation name>]

This will list the given relation name, or the relation name currently executing
if the parameter is omitted.

If you have real problems that need a more complex interface than this, can you
please ping me online? I'd be happy to understand them.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:21: possible to filter that as desired without
using the `--interface`
So why do we need it?

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:25: verify the continued existence of this
relation id for the
That's relation-ids | grep $R

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:31: script. Calling `relation-info` for other
services is read at that
Why do we need to ask about relation ids for other services? Nothing in juju
allows the local unit to fiddle with relation names and ids from remote services
or units.

[jimbaker, 2012-03-20 20:00:47]

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info.rst
File source/drafts/relation-info.rst (right):

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:7: relation-info [--interface INTERFACE] [-r
RELATION_NAME_OR_ID]
This is a nice simplification. What I originally proposed was to use a parallel
arg/option structure to the other relation hook commands. But this works much
better.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:21: possible to filter that as desired without
using the `--interface`
This part of the proposal was out of date. The original proposed format for the
relation id used <interface>-<normalized internal relation id>. Now it's
<relation name>:<normalized internal relation id>. So we may still want to
support --interface, per the Keystone example in this proposal.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:25: verify the continued existence of this
relation id for the
Sounds good to me.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:31: script. Calling `relation-info` for other
services is read at that
Fair enough.

[jimbaker, 2012-03-22 04:15:29]

Please take a look.

[jimbaker, 2012-03-22 04:23:54]

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info.rst
File source/drafts/relation-info.rst (right):

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:7: relation-info [--interface INTERFACE] [-r
RELATION_NAME_OR_ID]
On 2012/03/20 19:47:36, niemeyer wrote:
> If this command enumerates relation ids, how can it accept a relation id as a
> parameter? Also, I don't see how it can take a service name as a parameter
> either, or why it takes an interface, or even a unit name.
> 
> If I understand what this command is doing, I'd imagine something much simpler
> instead:
> 
>     relation-ids [<relation name>]
> 
> This will list the given relation name, or the relation name currently
executing
> if the parameter is omitted.
> 
> If you have real problems that need a more complex interface than this, can
you
> please ping me online? I'd be happy to understand them.

Done.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:21: possible to filter that as desired without
using the `--interface`
Simply removed interface discussion, since no real use case, per irc discussion.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:25: verify the continued existence of this
relation id for the
On 2012/03/20 20:00:47, jimbaker wrote:
> Sounds good to me.

Done.

https://codereview.appspot.com/5837050/diff/3001/source/drafts/relation-info....
source/drafts/relation-info.rst:31: script. Calling `relation-info` for other
services is read at that
On 2012/03/20 20:00:47, jimbaker wrote:
> Fair enough.

Done.

[hazmat, 2012-03-27 12:47:14]

LGTM, some minors.

https://codereview.appspot.com/5837050/diff/9001/source/drafts/relation-info.rst
File source/drafts/relation-info.rst (right):

https://codereview.appspot.com/5837050/diff/9001/source/drafts/relation-info....
source/drafts/relation-info.rst:16: For any hook, the data for the
`relation-ids` command is cached prior
This feels like an internal impl detail, i suggest its removal.

https://codereview.appspot.com/5837050/diff/9001/source/drafts/relation-info....
source/drafts/relation-info.rst:36: list of relation ids that provide the
keystone interface, then
"of the relation ids for the relation instances with the keystone-service name
and keystone service interface."

The comma here should be a period. The relation instances provide the interface.

https://codereview.appspot.com/5837050/diff/9001/source/drafts/relation-info....
source/drafts/relation-info.rst:37: enumerated over to set the ready for each
relation. Assume that
This part after the comma doesn't make any sense to me, i'd suggest its removal.

"then enumerated over to set the ready for each relation"