cf-charmers team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

So this is reviewed in the context of a charm, but its nice to see the
top level singleton thing play out.

I have some more bikeshedding suggestions, but they are only that. Feel
free to land this and the helpers changes. They

LGTM +1

Thanks


https://codereview.appspot.com/100680044/diff/40001/hooks/charmhelpers/core/services.py
File hooks/charmhelpers/core/services.py (right):

https://codereview.appspot.com/100680044/diff/40001/hooks/charmhelpers/core/services.py#newcode9
hooks/charmhelpers/core/services.py:9: class ServiceManager(object):
Even though we only use this as a Singleton this is almost certainly the
proper way to evolve the code. Better to do this now (because its a
minor api change) than later, thanks.

https://codereview.appspot.com/100680044/diff/40001/hooks/charmhelpers/core/services.py#newcode21
hooks/charmhelpers/core/services.py:21: "data_lost": <one or more
callbacks>,
Not sure I love data_ready/data_lost. Still I feel like I'm bike
shedding and whit already reviewed this and was ok with the naming so
I'll pass if you don't like these better

'requires': [],
'complete': []
'incomplete': []

another option is to keep with the naming we use in relations though
I'll admit 'joined' is a little hard to understand in this context, so
maybe

'requires', 'changed', 'broken'

I do think 'broken' is a good name, but it needs something that fits
conceptually with it. At this point though I do think what you have is
understandable and nicely documented.

https://codereview.appspot.com/100680044/diff/40001/hooks/config.py
File hooks/config.py (right):

https://codereview.appspot.com/100680044/diff/40001/hooks/config.py#newcode8
hooks/config.py:8:
Yeah, not bad.

https://codereview.appspot.com/100680044/

-- 
https://code.launchpad.net/~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu/+merge/220741
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu into lp:~cf-charmers/charms/trusty/cf-cloud-controller/trunk.