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

Reviewers: mp+97721_code.launchpad.net,

Message:
Please take a look.

Description:

https://code.launchpad.net/~jimbaker/juju/relation-reference-spec/+merge/97721

(do not edit description out of merge proposal)

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

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

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

Index: source/drafts/relation-reference.rst
=== added file 'source/drafts/relation-reference.rst'
--- source/drafts/relation-reference.rst 1970-01-01 00:00:00 +0000
+++ source/drafts/relation-reference.rst 2012-03-15 18:53:17 +0000
@@ -0,0 +1,46 @@
+Relation references
+-------------------
+
+A number of use cases in Juju require explicit reference to
+a relation. Such references should be supported by either using the
+relation name or the relation id:
+
+ * The **relation name** is currently visible in Juju: it's a
+ component of relation hook script names and corresponding
+ `metadata.yaml`, and it's made available as the environment
+ variable `$JUJU_RELATION` to relation hook scripts.
+
+ * Currently the **relation id** is not visible to user scripts, but
+ an internal version is used by Juju itself. With this proposal, a
+ new environment variable, `$JUJU_RELATION_ID`, is defined for
+ relation hooks.
+
+Relation ids are also sometimes required. This is because a service
+may **provide** an interface to multiple consuming services with the
+same relation name; each of these is a different established
+relation. For example the ``mysql`` service might provide the
+``mysql`` interface to multiple consuming clients using the relation
+name ``db``. In this scenario, to specify a relation, it's necessary
+to use its relation id, not the relation name.
+
+Relation ids can be looked up by `relation-info`, a new hook command,
+as described in a subsequent section.
+
+To ensure readibility of this relation id, its format is specified to
+be ``<interface name>-<normalized internal relation
+id>``. (Normalization removes any padding zeros from the internal id.)
+This format was mentioned in `bug #767195
+https://bugs.launchpad.net/juju/+bug/767195`_.
+
+Example: if the relation is associated with interface ``http`` and the
+specific interal relation id is ``relation-0000000042``, then the
+relation id is ``http-42``.
+
+As a consequence of this format change, the following restriction is
+made on relation names so they may always be distinguished from
+relation ids: a relation name is invalid if it ends with a hyphen
+followed by a number.
+
+Lastly, relation ids are never visible outside of relation hooks or
+relation hook commands. In particular, they are not seen in juju
+status output.

Reviewers: mp+97721_code.launchpad.net,

Message:
Please take a look.

Description:
Add support for using relation ids to refer unambiguously to relations.

https://code.launchpad.net/~jimbaker/juju/relation-reference-spec/+merge/97721

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   A source/drafts/relation-reference.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-reference.rst
=== added file 'source/drafts/relation-reference.rst'
--- source/drafts/relation-reference.rst 1970-01-01 00:00:00 +0000
+++ source/drafts/relation-reference.rst 2012-03-15 18:53:17 +0000
@@ -0,0 +1,46 @@
+Relation references
+-------------------
+
+A number of use cases in Juju require explicit reference to
+a relation. Such references should be supported by either using the
+relation name or the relation id:
+
+ * The **relation name** is currently visible in Juju: it's a
+ component of relation hook script names and corresponding
+ `metadata.yaml`, and it's made available as the environment
+ variable `$JUJU_RELATION` to relation hook scripts.
+
+ * Currently the **relation id** is not visible to user scripts, but
+ an internal version is used by Juju itself. With this proposal, a
+ new environment variable, `$JUJU_RELATION_ID`, is defined for
+ relation hooks.
+
+Relation ids are also sometimes required. This is because a service
+may **provide** an interface to multiple consuming services with the
+same relation name; each of these is a different established
+relation. For example the ``mysql`` service might provide the
+``mysql`` interface to multiple consuming clients using the relation
+name ``db``. In this scenario, to specify a relation, it's necessary
+to use its relation id, not the relation name.
+
+Relation ids can be looked up by `relation-info`, a new hook command,
+as described in a subsequent section.
+
+To ensure readibility of this relation id, its format is specified to
+be ``<interface name>-<normalized internal relation
+id>``. (Normalization removes any padding zeros from the internal id.)
+This format was mentioned in `bug #767195
+https://bugs.launchpad.net/juju/+bug/767195`_.
+
+Example: if the relation is associated with interface ``http`` and the
+specific interal relation id is ``relation-0000000042``, then the
+relation id is ``http-42``.
+
+As a consequence of this format change, the following restriction is
+made on relation names so they may always be distinguished from
+relation ids: a relation name is invalid if it ends with a hyphen
+followed by a number.
+
+Lastly, relation ids are never visible outside of relation hooks or
+relation hook commands. In particular, they are not seen in juju
+status output.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst
File source/drafts/relation-reference.rst (right):

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode6
source/drafts/relation-reference.rst:6: relation name or the relation
id:
You need to back these statements. The proposal starts by already saying
that the feature is required, and that it's going to be required in one
of two ways. I suggest dropping part of this introduction and starting
the spec at the point mentioned below.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode18
source/drafts/relation-reference.rst:18: Relation ids are also sometimes
required. This is because a service
Start at "[A] service may provide an interface (...)". Here you have a
good introduction to why you're proposing a change to the current
mechanism.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode22
source/drafts/relation-reference.rst:22: ``mysql`` interface to multiple
consuming clients using the relation
Examples benefit from unique names so that one can tell what is being
referred to:

     (...). For example, the ``mysql5`` service might provide
     a relation named `db` with a ``mysql`` interface.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode24
source/drafts/relation-reference.rst:24: to use its relation id, not the
relation name.
This sentence has to be more specific. In many circumstances, it is
possible to specify a relation solely by its name, even in the scenario
you provide. Note that so far we don't have this setting, and we manage
to do reasonably well without it. Please finish this paragraph with an
actual example that requires such unique id, to give context to the
whole proposal.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode27
source/drafts/relation-reference.rst:27: as described in a subsequent
section.
Also needs context. Why is this necessary? You've also mentioned
$JUJU_RELATION_ID above. What's the difference between them?

The paragraph also needs to be moved. It's saying how a relation id may
be obtained, without ever describing what it is, which is done by the
following paragraph.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode30
source/drafts/relation-reference.rst:30: be ``<interface
name>-<normalized internal relation
This should probably be <relation name> rather than <interface name>
and, it'd be good to avoid the dash between those entries, because
relation names are often named after the charm, and "mysql-42" looks
like a charm name-revision. Might also look nicer if indented.

I suggest something like:

"""
To ensure readability of this relation id, its format is specified to
be::

     <relation name>:<normalized internal relation id>

Normalization removes any padding zeros from the internal id. For
example, if the relation is associated with a relation named ``http``
and the respective relation id is ``relation-0000000042``, then the
relation id is ``http:42``.
"""

Please put the bug ...

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst
File source/drafts/relation-reference.rst (right):

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode6
source/drafts/relation-reference.rst:6: relation name or the relation
id:
Done.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode18
source/drafts/relation-reference.rst:18: Relation ids are also sometimes
required. This is because a service
On 2012/03/15 20:19:48, niemeyer wrote:
> Start at "[A] service may provide an interface (...)". Here you have a
good
> introduction to why you're proposing a change to the current
mechanism.

Done.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode22
source/drafts/relation-reference.rst:22: ``mysql`` interface to multiple
consuming clients using the relation
On 2012/03/15 20:19:48, niemeyer wrote:
> Examples benefit from unique names so that one can tell what is being
referred
> to:

> (...). For example, the ``mysql5`` service might provide
> a relation named `db` with a ``mysql`` interface.

Done.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode24
source/drafts/relation-reference.rst:24: to use its relation id, not the
relation name.
On 2012/03/15 20:19:48, niemeyer wrote:
> This sentence has to be more specific. In many circumstances, it is
possible to
> specify a relation solely by its name, even in the scenario you
provide. Note
> that so far we don't have this setting, and we manage to do reasonably
well
> without it. Please finish this paragraph with an actual example that
requires
> such unique id, to give context to the whole proposal.

Done.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode27
source/drafts/relation-reference.rst:27: as described in a subsequent
section.
On 2012/03/15 20:19:48, niemeyer wrote:
> Also needs context. Why is this necessary? You've also mentioned
> $JUJU_RELATION_ID above. What's the difference between them?

> The paragraph also needs to be moved. It's saying how a relation id
may be
> obtained, without ever describing what it is, which is done by the
following
> paragraph.

Done.

https://codereview.appspot.com/5836049/diff/1/source/drafts/relation-reference.rst#newcode30
source/drafts/relation-reference.rst:30: be ``<interface
name>-<normalized internal relation
On 2012/03/15 20:19:48, niemeyer wrote:
> This should probably be <relation name> rather than <interface name>
and, it'd
> be good to avoid the dash between those entries, because relation
names are
> often named after the charm, and "mysql-42" looks like a charm
name-revision.
> Might also look nicer if indented.

> I suggest something like:

> """
> To ensure readability of this relation id, its format is specified to
be::

> <relation name>:<normalized internal relation id>

> Normalization removes any padding zeros from the internal id. For
example, if
> the relation is associated with a relation named ``http`` and the
respective
> relation id is ``relation-0000000042``, then the relation id is
``ht...

Please take a look.

https://codereview.appspot.com/5836049/

Much better, thank you.

Just a couple of final details.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst
File source/drafts/relation-reference.rst (right):

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode2
source/drafts/relation-reference.rst:2: -------------------
Please check the other specs to see how they format headers and use the
same syntax. I believe it's === for the first header and --- for the
internal ones.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode35
source/drafts/relation-reference.rst:35: its relation id, not the
relation name. However, the use of relation
Remove content after the dot. We'll use it later.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode46
source/drafts/relation-reference.rst:46: addition, a new environment
variable, `$JUJU_RELATION_ID`, will always
In addition to normalization we're setting an environment variable?
That's a different topic. Break line, and bring back the earlier text
merged with it:

"""
Right now relation ids are only visible inside relation hooks, via the
$JUJU_RELATION_ID environment variable. In particular, they are not
visible in the output of "juju status".
"""

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode62
source/drafts/relation-reference.rst:62: ~~~~~~~~~~
I believe we use --- for internal headers.

https://codereview.appspot.com/5836049/

Please take a look.

https://codereview.appspot.com/5836049/

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst
File source/drafts/relation-reference.rst (right):

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode2
source/drafts/relation-reference.rst:2: -------------------
On 2012/03/20 13:31:22, niemeyer wrote:
> Please check the other specs to see how they format headers and use
the same
> syntax. I believe it's === for the first header and --- for the
internal ones.

Done.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode35
source/drafts/relation-reference.rst:35: its relation id, not the
relation name. However, the use of relation
On 2012/03/20 13:31:22, niemeyer wrote:
> Remove content after the dot. We'll use it later.

Done.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode46
source/drafts/relation-reference.rst:46: addition, a new environment
variable, `$JUJU_RELATION_ID`, will always
On 2012/03/20 13:31:22, niemeyer wrote:
> In addition to normalization we're setting an environment variable?
That's a
> different topic. Break line, and bring back the earlier text merged
with it:

> """
> Right now relation ids are only visible inside relation hooks, via the
> $JUJU_RELATION_ID environment variable. In particular, they are not
visible in
> the output of "juju status".
> """

Done.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode62
source/drafts/relation-reference.rst:62: ~~~~~~~~~~
On 2012/03/20 13:31:22, niemeyer wrote:
> I believe we use --- for internal headers.

Done.

https://codereview.appspot.com/5836049/

You haven't actually done the changes. I don't care much, though. The
meat of the spec look good.

LGTM

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst
File source/drafts/relation-reference.rst (right):

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode35
source/drafts/relation-reference.rst:35: its relation id, not the
relation name. However, the use of relation
On 2012/03/22 04:17:53, jimbaker wrote:
> On 2012/03/20 13:31:22, niemeyer wrote:
> > Remove content after the dot. We'll use it later.

> Done.

Not done.

https://codereview.appspot.com/5836049/diff/5001/source/drafts/relation-reference.rst#newcode46
source/drafts/relation-reference.rst:46: addition, a new environment
variable, `$JUJU_RELATION_ID`, will always
On 2012/03/22 04:17:53, jimbaker wrote:
> On 2012/03/20 13:31:22, niemeyer wrote:
> > In addition to normalization we're setting an environment variable?
That's a
> > different topic. Break line, and bring back the earlier text merged
with it:
> >
> > """
> > Right now relation ids are only visible inside relation hooks, via
the
> > $JUJU_RELATION_ID environment variable. In particular, they are not
visible in
> > the output of "juju status".
> > """

> Done.

Not done.

https://codereview.appspot.com/5836049/

merged into lp:juju/docs instead