cf-charmers team mailing list archive

[Cory Johns <cory.johns@xxxxxxxxxxxxx>]

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 of
the default
> unit.
> There is a rule I try to use when generating/process data these days,
no data
> should live in the key side of the mapping. In the context of a
template for
> iteration is simpler to iterate

> context[interface][{_unit: 'unit/0', 'foo': 'bar'},
>                     {_unit: 'unit/1', 'foo': bar}]

> This rule makes additional sense when thinking that knowing the unit
names to
> request them would imply a broken charm and thus shouldn't be an index
in this
> context. I'd suggest transitioning to that. We could also make the
rule (and
> document here) that only elements with complete data appear in the
list.

Can you explain your reasoning behind making use of the key?  I'm not
averse to changing it to being a list instead of a mapping, but using a
"special" field to hold the unit name ('_unit', in your example) seems
more dangerous, since it could potentially conflict with actual fields,
even if unlikely due to the underscore prefix.

With your comment that the charm shouldn't actually know / care about
the unit name, maybe we can just leave it out altogether, especially if
it only contains "complete" data sets.

https://codereview.appspot.com/96680043/

-- 
https://code.launchpad.net/~johnsca/charm-helpers/multi-unit/+merge/221801
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charm-helpers/multi-unit into lp:~cf-charmers/charm-helpers/cloud-foundry.