Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1458)

Issue 5714043: Modified support for working with relations

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by Jim Baker
Modified:
9 years, 6 months ago
Reviewers:
mp+95268, hazmat, niemeyer
Visibility:
Public.

Description

This proposal pulls together a number of changes, backed by use cases from bug reports, for modifying Juju's current relation support as follows: * Enable working with relation ids. This includes adding a new environment variable `$JUJU_RELATION_ID` for relation hooks. * 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. * Add a new relation hook command, `relation-info`, for listing the relation ids associated with a service. * Fix a bug in `juju status` status for the scenario that a service has multiple established relations to consuming clients, each using the same relation name from the provider's perspective. This fix does introduce a backwards breaking change. https://code.launchpad.net/~jimbaker/juju/enhanced-relation-spec/+merge/95268 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 22

Patch Set 2 : Modified support for working with relations #

Unified diffs Side-by-side diffs Delta from patch set Stats (+323 lines, -0 lines) Patch
A source/drafts/relation-support-changes.rst View 1 1 chunk +323 lines, -0 lines 0 comments Download

Messages

Total messages: 5
Jim Baker
Please take a look.
9 years, 7 months ago (2012-02-29 21:41:25 UTC) #1
hazmat
Needs Fixing + Needs Information. http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations.rst File source/drafts/enhanced-relations.rst (right): http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations.rst#newcode15 source/drafts/enhanced-relations.rst:15: I'd switch this to.. ...
9 years, 7 months ago (2012-03-01 05:42:08 UTC) #2
hazmat
more comments from joint discussion. http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations.rst File source/drafts/enhanced-relations.rst (right): http://codereview.appspot.com/5714043/diff/1/source/drafts/enhanced-relations.rst#newcode16 source/drafts/enhanced-relations.rst:16: * Enable `relation-get`, `relation-set`, ...
9 years, 7 months ago (2012-03-01 19:48:08 UTC) #3
Jim Baker
Please take a look.
9 years, 6 months ago (2012-03-07 05:43:51 UTC) #4
niemeyer
9 years, 6 months ago (2012-03-15 18:58:38 UTC) #5
Discussed on IRC:

<niemeyer> jimbaker: Sure, will review it onw
<niemeyer> now
<jimbaker> niemeyer, thanks
<niemeyer> jimbaker: This looks a bit like a bag-of-changes spec.. these changes
being described in the summary look quite unrelated to each other
<jimbaker> niemeyer, ok
<niemeyer> jimbaker: There's a pretty clear hint of that in the topic of the
spec actually: "Modified support for working with relations"
* fwereade__ has quit (Ping timeout: 260 seconds)
<jimbaker> niemeyer, you are absolutely right about that title, it was very
intentional
<niemeyer> jimbaker: As in, intentionally vague? Why?
<jimbaker> niemeyer, this is what i was asked to work on
<jimbaker> nonetheless, there's only this common theme
<jimbaker> that it is working on a number of features about relations
<niemeyer> jimbaker: on something intentionally vague? Sorry, I'm not following
what you mean..
<niemeyer> jimbaker: Yeah, but you being asked to work on a number of things is
something unrelated to having a spec that describes a number of unrelated things
<niemeyer> jimbaker: We can reach a different agreement on each one of those
things
<niemeyer> jimbaker: and this single spec will be blocked while we can't reach
an agreement on all of them
<jimbaker> it's not meant to be intentionally vague, but it does cover a range
of useful topics
<jimbaker> if you want to consider it a set of separate proposals, that probably
works
<jimbaker> and we can work on each one on in turn
<niemeyer> jimbaker: I'll go over it because I guess that's the shortest path to
action by now, but it's better to have more clearly defined specs and goals. You
could have some of those bits already done by now, for example, if you had
started with a single one of them
<jimbaker> niemeyer, ok
<niemeyer> jimbaker: Actually, that's not true..
<niemeyer> jimbaker: We can probably get approval on some of those aspects very
quickly
<niemeyer> jimbaker: and you might start working on them right away
<jimbaker> niemeyer, that's good to hear
<niemeyer> jimbaker: Can you please do something very quick: take those
independent concerns apart and put them in separate branches, and repush for
review
<jimbaker> niemeyer, ok
* Leseb has quit (Quit: Leseb)
<niemeyer> jimbaker: I'll review all of them in turn right away
<niemeyer> jimbaker: But I expect we'll be able to get some of them in quickly,
while others will require more back and forth
<niemeyer> jimbaker: You can work on the initial ones first, while we reach
agreement in the rest
<jimbaker> niemeyer, sounds good
<niemeyer> jimbaker: Thanks, and sorry for the trouble
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b