Callbacks instead of ServiceManagers

Callbacks instead of ServiceManagers

Refactored charmhelpers.core.services to use callbacks instead of
service types / managers, to make it easier for charms to specify
custom behavior.

https://code.launchpad.net/~johnsca/charm-helpers/services-callback-fu/+merge/220733

(do not edit description out of merge proposal)

[johnsca, 2014-05-22 22:54:16]

Please take a look.

[benjamin.saller, 2014-05-22 23:47:43]

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#newc...
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#...
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#...
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#...
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#...
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.

[johnsca, 2014-05-23 13:46:55]

On 2014/05/22 23:47:43, benjamin.saller wrote:
> We should also consider an 'incomplete' event, if a relation is broken we
might
> stop the service for example.

> 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.

I wonder if we should change "complete" to "start," since what we're really
saying is "this code should run when the service is ready to start."  Then,
"stop" can be called if stop_services() is called or if a relationship gets
broken such that the service no longer has all of the valid information ("this
code should run when the service has to stop, for whatever reason").


> Handle single argument string calls, 

https://codereview.appspot.com/98490043/diff/1/tests/core/test_services.py#ne...

`foo(*args)` is a "varargs" definition.   It will accept `foo()`, `foo('bar')`,
`foo('bar', 'baz')`, etc. (but not `params = ['bar', 'baz']; foo(params)`; that
would have to be written `params = ['bar', 'baz']; foo(*params)`), and `args`
will automatically be a list.

[johnsca, 2014-05-26 18:50:34]

Please take a look.

[johnsca, 2014-05-26 21:18:16]

Please take a look.

[johnsca, 2014-05-27 17:27:31]

Please take a look.

[benjamin.saller, 2014-05-27 18:07:24]

Comments for this went into the charm, but this LGTM as well, thanks.

[johnsca, 2014-05-27 21:18:59]

Please take a look.

[johnsca, 2014-05-29 17:24:30]

Please take a look.

[benjamin.saller, 2014-05-29 17:40:21]

Thanks for tracking down the err0r, and yes, explicit win (esp. since we hit
this in practice). 

+1

[johnsca, 2014-05-29 20:17:58]

*** Submitted:

Callbacks instead of ServiceManagers

Refactored charmhelpers.core.services to use callbacks instead of
service types / managers, to make it easier for charms to specify
custom behavior.

R=benjamin.saller
CC=
https://codereview.appspot.com/98490043