Merge lp://staging/~jimbaker/pyjuju/relation-hook-commands-spec into lp://staging/~juju/pyjuju/docs

Reviewers: mp+97733_code.launchpad.net,

Message:
Please take a look.

Description:
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)

Please review this at https://codereview.appspot.com/5836050/

Affected files:
   A [revision details]
   A source/drafts/relation-hook-commands.rst

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: source/drafts/relation-hook-commands.rst
=== added file 'source/drafts/relation-hook-commands.rst'
--- source/drafts/relation-hook-commands.rst 1970-01-01 00:00:00 +0000
+++ source/drafts/relation-hook-commands.rst 2012-03-15 19:34:59 +0000
@@ -0,0 +1,62 @@
+Commands to work with relation settings and membership
+------------------------------------------------------
+
+The hook commands working with relation settings (`relation-get`,
+`relation-set`, and `relation-list`) are modified to enable their use
+with other relations than the implied relation and also for other
+hooks besides relation hooks::
+
+ relation-get [-r RELATION_NAME_OR_ID] [-|SETTING] [UNIT_NAME]
+
+ relation-set [-r RELATION_NAME_OR_ID]
+ [SETTING=VALUE [SETTING=VALUE ...]] [UNIT_NAME]
+
+ relation-list [-r RELATION_NAME_OR_ID] [UNIT_NAME]
+
+The current behavior for these commands is to use the **implied
+relation**. This relation is the one in effect when the relation hook
+command is used in a relation hook. When not running in the context of
+a relation hook and its implied relation, it is necessary to specify a
+relation unambiguously. Note that there can be several possible
+implied relations for a given relation name, in which case the
+relation id *must* be used to specify the desired relation.
+
+Example. To get the user name from the relation named `db` from
+`mysql/0` from any hook::
+
+ user=`relation-get -r db user mysql/0`
+
+For the relational hook commands `relation-get`, `relation-set`, and
+`relation-list`, the following apply for non-implied relations when
+run from any hook:
+
+ * Referring to a relation causes its hook context to be instantiated
+ and cached. Subsequent reads of settings and the relation listing
+ will return the same values for the duration of the hook calling
+ these hook commands ("consistency").
+
+ * If a relation name is specified with `-r` and the relation cannot
+ be precisely determined by this name, an error is raised stating
+ "Ambiguous relation name".
+
+ * If the relation does not exist at the time of read/caching, an
+ error is raised stating "Relation not found".
+
+ * If the hook exits with status code 0, the hook contexts are then
+ written into ZooKeeper, one at a ti...

Please take a look.

https://codereview.appspot.com/5836050/

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

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-commands.rst#newcode1
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-commands.rst#newcode2
source/drafts/relation-hook-commands.rst:2:
------------------------------------------------------
Please fix the header formats.

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-commands.rst#newcode45
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-commands.rst#newcode70
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-commands.rst#newcode79
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?

https://codereview.appspot.com/5836050/

Please take a look.

https://codereview.appspot.com/5836050/

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

https://codereview.appspot.com/5836050/diff/2001/source/drafts/relation-hook-commands.rst#newcode1
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-commands.rst#newcode2
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-commands.rst#newcode45
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-commands.rst#newcode70
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-commands.rst#newcode79
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.

https://codereview.appspot.com/5836050/

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-commands.rst
File source/drafts/relation-hook-commands.rst (right):

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-commands.rst#newcode22
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-commands.rst#newcode47
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-commands.rst#newcode56
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.

https://codereview.appspot.com/5836050/

Please take a look.

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

https://codereview.appspot.com/5836050/diff/6001/source/drafts/relation-hook-commands.rst#newcode22
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-commands.rst#newcode47
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-commands.rst#newcode56
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.

https://codereview.appspot.com/5836050/

Fixed in the latest MP

https://codereview.appspot.com/5836050/

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

https://codereview.appspot.com/5836050/

merged into lp:~charm-contributors/juju/docs instead