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

Issue 96680043: Refactored RelationContext multi-unit support

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by johnsca
Modified:
11 years ago
Reviewers:
benjamin.saller, mp+221801
Visibility:
Public.

Description

Refactored RelationContext multi-unit support Added a little magic to the RelationContext to make it better able to support multiple units and still DWIM in both cases. This might be too magical, though. https://code.launchpad.net/~johnsca/charm-helpers/multi-unit/+merge/221801 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Refactored RelationContext multi-unit support #

Total comments: 2

Patch Set 3 : Refactored RelationContext multi-unit support #

Patch Set 4 : Refactored RelationContext multi-unit support #

Patch Set 5 : Refactored RelationContext multi-unit support #

Total comments: 4

Patch Set 6 : Refactored RelationContext multi-unit support #

Total comments: 2

Patch Set 7 : Refactored RelationContext multi-unit support #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+106 lines, -81 lines) Patch
A [revision details] View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M charmhelpers/contrib/cloudfoundry/contexts.py View 1 2 3 4 5 3 chunks +17 lines, -6 lines 0 comments Download
M charmhelpers/core/services.py View 1 2 3 4 5 6 4 chunks +72 lines, -53 lines 1 comment Download
M tests/contrib/cloudfoundry/test_render_context.py View 1 2 3 4 5 2 chunks +8 lines, -9 lines 0 comments Download
M tests/core/test_services.py View 1 2 3 4 5 3 chunks +7 lines, -13 lines 0 comments Download

Messages

Total messages: 15
johnsca
Please take a look.
11 years ago (2014-06-02 22:16:30 UTC) #1
johnsca
Please take a look.
11 years ago (2014-06-03 16:02:01 UTC) #2
benjamin.saller
Thanks for this. I include some thoughts below, I hope they are agreeable. https://codereview.appspot.com/96680043/diff/20001/charmhelpers/core/services.py File ...
11 years ago (2014-06-04 20:57:39 UTC) #3
johnsca
On 2014/06/04 20:57:39, benjamin.saller wrote: > https://codereview.appspot.com/96680043/diff/20001/charmhelpers/core/services.py#newcode298 > charmhelpers/core/services.py:298: over the units, not the items ...
11 years ago (2014-06-04 21:09:21 UTC) #4
johnsca
That should be: "behind *not* making use of the key"
11 years ago (2014-06-04 21:10:01 UTC) #5
benjamin.saller
On 2014/06/04 21:09:21, johnsca wrote: > On 2014/06/04 20:57:39, benjamin.saller wrote: > > > https://codereview.appspot.com/96680043/diff/20001/charmhelpers/core/services.py#newcode298 ...
11 years ago (2014-06-04 21:22:03 UTC) #6
johnsca
Please take a look.
11 years ago (2014-06-05 17:36:34 UTC) #7
johnsca
Please take a look.
11 years ago (2014-06-05 17:48:44 UTC) #8
johnsca
Please take a look.
11 years ago (2014-06-06 16:07:44 UTC) #9
benjamin.saller
Thanks for this, very close. Mostly this comes down to me wanting another pass over ...
11 years ago (2014-06-06 16:43:12 UTC) #10
johnsca
On 2014/06/06 16:43:12, benjamin.saller wrote: > https://codereview.appspot.com/96680043/diff/80001/charmhelpers/contrib/cloudfoundry/contexts.py#newcode36 > charmhelpers/contrib/cloudfoundry/contexts.py:36: required_keys = ['address', > 'port', 'user', ...
11 years ago (2014-06-06 17:41:11 UTC) #11
johnsca
Please take a look.
11 years ago (2014-06-06 18:26:43 UTC) #12
benjamin.saller
Thanks, notes follow. https://codereview.appspot.com/96680043/diff/100001/charmhelpers/core/services.py File charmhelpers/core/services.py (right): https://codereview.appspot.com/96680043/diff/100001/charmhelpers/core/services.py#newcode259 charmhelpers/core/services.py:259: order: 'wordpress/0', 'wordpress/1', 'mediawiki/0'. I think ...
11 years ago (2014-06-06 18:48:31 UTC) #13
johnsca
Please take a look.
11 years ago (2014-06-06 19:44:16 UTC) #14
benjamin.saller
11 years ago (2014-06-06 19:45:47 UTC) #15
+1 LTGM, thanks for talking through that issue.

https://codereview.appspot.com/96680043/diff/120001/charmhelpers/core/service...
File charmhelpers/core/services.py (right):

https://codereview.appspot.com/96680043/diff/120001/charmhelpers/core/service...
charmhelpers/core/services.py:272: set of data came from, you'll need to extend
this class to preserve
Great, thanks
Sign in to reply to this message.

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