canonical / charms.reactive

Framework for developing charms and relations using the reactive pattern
Apache License 2.0
21 stars 34 forks source link

Add @action decorator #11

Open johnsca opened 8 years ago

johnsca commented 8 years ago

We should support reactive actions. The first step to this is having an @action decorator analogous to @hook.

bcsaller commented 8 years ago

The thinking here is unclear. Would an action being run call code that discovered all the handlers for that action? How do you merge output from the various functions? If we are attempting to guard on state we might want to do this a different way.

johnsca commented 8 years ago

We don't need to merge any output; handlers can either call action-set or not.

And yes, the assumption implicit in the issue is that all handlers would be discovered and potentially triggered, though of course @hook handlers would be excluded in favor of the appropriate @action handler. It would definitely require some documentation and education to ensure awareness that handlers could be triggered in actions as well as hook, but I don't think it's necessarily a blocker.

stub42 commented 8 years ago

I would like to add an 'upgrade' action to my apt layer.

@action('upgrade')
def upgrade():
    reactive.set_state('apt.needs_upgrade')

@when('apt.needs_upgrade')
def do_upgrade():
    hookenv.status_set('maintenance', 'Upgrading packages')
    with apt.unhold_packages():
        apt.upgrade()
    reactive.remove_state('apt.needs_upgrade')
    hookenv.action_set(upgraded=True)
    hookenv.status_set('active', 'Packages upgraded')  # TODO: Should reset original status

The charm using the layer may add their own behavior, defining their own @action('upgrade') hook that, for example, shuts down daemons. It also can, with a little state juggling, organize a handler to run at the end (for example, restart daemons).

@action('upgrade')
def myupgrade():
     mystuff.stop()
     reactive.set_state('needs_restart_after_upgrade')

@when('needs_restart_after_upgrade')
@when_not('apt.needs_upgrade')
def restart_after_upgrade():
    if not mystuff.start():
        hookenv.action_fail('Failed to restart')
        hookenv.status_set('blocked', 'Failed to restart')

I like how actions and hooks can share the same handlers. Some care will certainly be needed writing more complex charms (for example, my action will need to call a low level 'stop right now' method rather than intiate a rolling restart chain of peer-relation-changed and leadership-settings-changed hooks).

stub42 commented 8 years ago

https://github.com/juju-solutions/charms.reactive/pull/66 implements this. I have use cases to implement in the PostgreSQL charm and the apt layer.