Enable relation hook commands to work with arbitrary relations.

Enable relation hook commands to work with arbitrary relations.

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.

https://code.launchpad.net/~jimbaker/juju/relation-hook-commands-spec/+merge/97733

(do not edit description out of merge proposal)

[jimbaker, 2012-03-15 19:42:06]

Please take a look.

[jimbaker, 2012-03-19 18:44:29]

Please take a look.

[niemeyer, 2012-03-21 21:00:04]

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
File source/drafts/relation-hook-commands.rst (right):

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:1: Commands to work with relation
settings and membership
Isn't this spec simply adding a -r option to relation?

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:2:
------------------------------------------------------
Please fix the header formats.

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:45: name ``server``. There may be
multiple such established relalation to
s/relalation/relations/

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:70: * If a relation name is specified
with `-r` and the relation cannot
This sounds very error prone. Having a command that seems to work with one
relation established but breaks with two will yield charms that seem to work but
break in the wild. Can we please support relation ids only for the moment?

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:79: when first originally instantiated.
In between this write and the
Why is the ordering important? Can't we leave that as an implementation detail?

[jimbaker, 2012-03-22 04:14:32]

Please take a look.

[jimbaker, 2012-03-22 04:21:00]

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
File source/drafts/relation-hook-commands.rst (right):

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:1: Commands to work with relation
settings and membership
In a nutshell, yes, along with some description of how relation hook contexts
are cached, then ultimately written, etc.

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:2:
------------------------------------------------------
On 2012/03/21 21:00:04, niemeyer wrote:
> Please fix the header formats.

Done.

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:45: name ``server``. There may be
multiple such established relalation to
Paragraph was removed since it was no longer necessary, given the use of only
relation ids, not relation names.

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:70: * If a relation name is specified
with `-r` and the relation cannot
On 2012/03/21 21:00:04, niemeyer wrote:
> This sounds very error prone. Having a command that seems to work with one
> relation established but breaks with two will yield charms that seem to work
but
> break in the wild. Can we please support relation ids only for the moment?

Done.

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:79: when first originally instantiated.
In between this write and the
On 2012/03/21 21:00:04, niemeyer wrote:
> Why is the ordering important? Can't we leave that as an implementation
detail?

Done.

[hazmat, 2012-03-27 13:00:55]

i'm a little concerned about the rel-id iteration that blows up, else the rest
is okay. The segues into impl details of other functionality seem extraneous
though.

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
File source/drafts/relation-hook-commands.rst (right):

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:22: ``db:42``, as obtained by using the
`relation-info` command,
should read relation-ids

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:47: error is raised stating "Relation
not found".
wouldn't that lead to a situation where you do relation-ids iterate through
doing a rel command and it blows up?

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:56: that such nodes are not
garbage-collected from ZooKeeper, even if
they wouldn't be garbage collectable till all the relations endpoint units are
gone, else their still in active use, and hence not garbage. its not really
ignored, because the correct way to inform the unit of this change, ie. the
execution of the broken hook is pending.

[jimbaker, 2012-04-09 20:48:33]

Please take a look.

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
File source/drafts/relation-hook-commands.rst (right):

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:22: ``db:42``, as obtained by using the
`relation-info` command,
On 2012/03/27 13:00:55, hazmat wrote:
> should read relation-ids

Done.

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:47: error is raised stating "Relation
not found".
Because both relation-ids for all relation names for the local service and the
corresponding contexts are now cached prior to invocation of the hook, it is not
possible for such a blow up to occur: there will be consistency between them.

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-...
source/drafts/relation-hook-commands.rst:56: that such nodes are not
garbage-collected from ZooKeeper, even if
Fair enough, I have removed this implementation note.

[jimbaker, 2012-04-09 20:49:22]

Fixed in the latest MP

[hazmat, 2012-04-11 12:14:13]

lgtm, pls move it out of drafts though when merging.