Adds support for -r option to specify relation id for relation hook commands.

Adds support for -r option to specify relation id for relation hook commands.

This branch builds on the previous branches for working with relation ids: -r specifies the relation id to lookup a relation hook context for relation-set, relation-get, and relation-list. In addition, there are some bug fixes for relation-ids around making relation name optional.

https://code.launchpad.net/~jimbaker/juju/relation-id-option/+merge/99456

(do not edit description out of merge proposal)

[jimbaker, 2012-03-27 01:14:04]

Please take a look.

[jimbaker, 2012-03-30 06:15:27]

Please take a look.

[hazmat, 2012-04-02 16:28:46]

the implementation here is feels a bit convoluted.

you have two apis on two different objects, one which is just a cache, which end
up stuffing one of those api objects onto another unrelated object which doesn't
even use it and is only required sometimes, all just to get a cache hit. this
feels like a hack.

i think this implementation needs to be reworked, there is only one invoker
active, with many hook contexts, instead of trying to add parent pointers to the
contexts, add a get invoker method to the api factory with a current execution
invoker accessor executor (ala get_hook_context).

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/commands.py
File juju/hooks/commands.py (right):

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/commands.py#newco...
juju/hooks/commands.py:105: default="",
shouldn't this still default to JUJU_RELATION? or is that still nonsensical?

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/invoker.py
File juju/hooks/invoker.py (right):

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/invoker.py#newcod...
juju/hooks/invoker.py:165: child.invoker = self
commenting this line out and running tests results in no errors..

https://codereview.appspot.com/5916045/diff/3001/juju/state/hook.py
File juju/state/hook.py (right):

https://codereview.appspot.com/5916045/diff/3001/juju/state/hook.py#newcode49
juju/state/hook.py:49: self._invoker = None
the property usage here doesn't seem to add any value vs. self.invoker = None

[jimbaker, 2012-04-05 03:43:12]

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/commands.py
File juju/hooks/commands.py (right):

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/commands.py#newco...
juju/hooks/commands.py:105: default="",
It was nonsensical, so changed accordingly in the upstream.

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/invoker.py
File juju/hooks/invoker.py (right):

https://codereview.appspot.com/5916045/diff/3001/juju/hooks/invoker.py#newcod...
juju/hooks/invoker.py:165: child.invoker = self
No more 'invoker' attribute so no longer applicable

https://codereview.appspot.com/5916045/diff/3001/juju/state/hook.py
File juju/state/hook.py (right):

https://codereview.appspot.com/5916045/diff/3001/juju/state/hook.py#newcode49
juju/state/hook.py:49: self._invoker = None
No more 'invoker' attribute so no longer applicable

[jimbaker, 2012-04-05 03:46:36]

Please take a look.

[hazmat, 2012-04-05 11:49:03]

looks much better thanks for refactoring, +1 with some minors.

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/executor.py
File juju/hooks/executor.py (right):

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/executor.py#newc...
juju/hooks/executor.py:261: This enables a lookup point for cached child
relation hook contexts.
^for the hook api.

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/protocol.py
File juju/hooks/protocol.py (right):

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/protocol.py#newc...
juju/hooks/protocol.py:462: self.invoker = invoker
please rename to invoker_provider/accessor to distinguish from actual invoker.

https://codereview.appspot.com/5916045/diff/10001/juju/state/hook.py
File juju/state/hook.py (right):

https://codereview.appspot.com/5916045/diff/10001/juju/state/hook.py#newcode48
juju/state/hook.py:48: # Invoker holding this context
There shouldn't be any invokers refs on the hook context anymore.

[jimbaker, 2012-04-05 17:01:30]

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/executor.py
File juju/hooks/executor.py (right):

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/executor.py#newc...
juju/hooks/executor.py:261: This enables a lookup point for cached child
relation hook contexts.
On 2012/04/05 11:49:03, hazmat wrote:
> ^for the hook api.

Done.

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/protocol.py
File juju/hooks/protocol.py (right):

https://codereview.appspot.com/5916045/diff/10001/juju/hooks/protocol.py#newc...
juju/hooks/protocol.py:462: self.invoker = invoker
On 2012/04/05 11:49:03, hazmat wrote:
> please rename to invoker_provider/accessor to distinguish from actual invoker.

Done.

https://codereview.appspot.com/5916045/diff/10001/juju/state/hook.py
File juju/state/hook.py (right):

https://codereview.appspot.com/5916045/diff/10001/juju/state/hook.py#newcode48
juju/state/hook.py:48: # Invoker holding this context
On 2012/04/05 11:49:03, hazmat wrote:
> There shouldn't be any invokers refs on the hook context anymore.

Done.