cf-charmers team mailing list archive
-
cf-charmers team
-
Mailing list archive
-
Message #00233
Re: Callbacks instead of ServiceManagers (issue 98490043)
LGTM with some trivials
Thanks for this.
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/host.py
File charmhelpers/core/host.py (right):
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/host.py#newcode67
charmhelpers/core/host.py:67: """Determine whether a system service is
available"""
'an OS service' or 'an init service' or 'an upstart service'
or something similar, service is very overloaded in this context
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py
File charmhelpers/core/services.py (right):
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode32
charmhelpers/core/services.py:32: restarting the service via Upstart).
Indicate that callback is called with service name,
We should also consider an 'incomplete' event, if a relation is broken
we might stop the service for example.
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode35
charmhelpers/core/services.py:35: the service via Upstart).
stop is fine, for a stop hook, but incomplete would be for
relation-broken hooks. We can defer that till we actually intend to
support it though.
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode94
charmhelpers/core/services.py:94: for service_name in service_names or
SERVICES.keys():
Handle single argument string calls,
if service_names and isinstance(service_names, str):
service_names = [service_names]
Makes sense with the 'incomplete' mentioned above
https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode104
charmhelpers/core/services.py:104: Given the name of a registered
service, return a ServiceManager
This isn't still a ServiceManager as per the comment if I'm reading this
properly.
https://codereview.appspot.com/98490043/
--
https://code.launchpad.net/~johnsca/charm-helpers/services-callback-fu/+merge/220733
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charm-helpers/services-callback-fu into lp:~cf-charmers/charm-helpers/cloud-foundry.
References