Actions for charms.reactive

[stub42]

This branch implements #11

It adds a new @action decorator. When the entry point is an action rather than a hook, the @action handlers are invoked instead of the @hook handlers. Nothing else changes - all the standard 'other' handlers will be invoked.

@action('act-a')
def test1(params):
    hookenv.action_set(dict(res1='test1'))
    hookenv.action_set(dict(parms=params))
    hookenv.action_set(dict(k1=params['k1']))
    hookenv.action_set(dict(k2=params['k2']))

This branch also pulls in my 'preflight' branch from #20 , so you might want to check that out in isolation. This branch also added a new phase to the main dispatcher, and seemed a good starting point and avoids conflicts. I think it will also be needed to fix juju-solutions/layer-basic#68 (if we can be bothered - it is pretty obscure and never seen in the wild).

I will also shortly be proposing pull request to the basic layer, adding an actions.template file to declare the standard entry point. It will also add a standard set of states, allowing you to write your action handlers with standard @when, and guard handlers that should not run during an action with @when_not. So, assuming you don't need your handler to run before the main handlers, you can declare your action like this:

@when('action.act-b')
def test2():
    hookenv.action_set(dict(res1='foo'))


@when('action.act-c')
def test3():
    hookenv.action_set(dict(res1='bar'))
    hookenv.action_fail('Whoops!!!!!')


@when('action.act-d')
def test3():
    raise RuntimeError('AAAARGH')

[johnsca]

Apologies for the long delay on commenting on this. I wasn't sure what direction would be right for handling actions and only just recently took the time to talk it over with @bcsaller.

On the one hand, I've found the @hook decorator to cause more problems than it solves, so my inclination was to avoid new decorators. For example, you're aware of the issue with @hook('config-changed') and the disconnect between the hook invocation and the data changing. There's also the issue with relation hooks and charm upgrades (since those relation hooks are not going to fire again, the relations may not have a way to get to the desired state). However, @bcsaller pointed out that, unlike hooks, actions are more like RPC calls in that they are on demand with a specific, discrete action desired, and all of the relevant input contained in the call itself. So I can now accept an @action decorator while still pushing to strongly discourage use of the @hook decorator.

I was also on the fence about the state-based dispatch of actions. I like the pattern of config and leadership value states, but @bcsaller pointed out that implicitly removed states artificially couples the reactive loop to a hook invocation which breaks down if we want to move the core of reactive out into a separate daemon to improve isolation / separation of concerns and language independence. Additionally, the name of the current action is fundamentally different than config values in that the action is really telling us explicitly which handler to invoke while config and leadership data is just part of the state of the system.

So, the upshot of all this is that I am in favor of this approach (with some specific comments that I'll note shortly) and would like to do away with the state portion of juju-solutions/layer-basic#69.

[johnsca]

Because of the RPC nature of actions that I mentioned in my previous comment, I think actions should skip the "other" phase entirely, so that only @setup and @action handlers are run.

We also need to add the @action decorator to the CLI portion.

[stub42]

Running the 'other' phase after the @action handler is the most compelling part of the design to me. If the 'other' phase isn't run, then there is little reason to couple actions with the reactive framework at all. I cannot share handlers between hooks and actions, and instead need to code my action in a linear sequence of operations the way we used to code hooks. The @setup handlers would be run, preparing the environment for a main event loop that will never be run. The action defined in my layer cannot trigger handlers defined in the charms using my layer.

Its worth mentioning that writing actions in this fashion is opt-in. The charmer needs to copy the template to the actions/action-name path (manually, or in the future by declaring it in layer.yaml and having charm build do it). If people want a self contained action that does not trigger reactive states they can continue to do so like they do today. They could even import charms.reactive and alter state of future hooks (we could provide helpers for that). What they can't do is, for example, change the service's administrative password and alter the reactive state causing the existing handlers to kick in and publish the updated password everywhere. And they especially can't do that if the action is defined in a layer and the handlers that need to be invoked defined in a charm using that layer.

The state portion in juju-solutions/layer-basic#69 can certainly be done away with, as to my mind it just provides an alternate spelling (and could be implemented in a layer with a @setup handler if people really preferred that spelling).

[johnsca]

I see your point about the charm potentially needing to react to state changes made by an action.

As far as reactive actions being opt-in, my inclination would be to modify charm-build to populate these templates by default and make them opt-out / overridable instead. I think that's more in line with what people would expect from actions in the reactive / layered framework.

[johnsca]

It just occurred to me that one of the goals of integrating actions with reactive is to be able to gate or choose action implementation based on state which would imply that we'd want @action to work with @when etc. Currently, there's the outstanding bug of @when and @hook not working together because of the phase issue. I think we might need to figure out if we can address that if we're going to go this route.

[marcoceppi]

It seems we'd want to transition @hook and maybe @action to @event where
@event is an artifical @When state

@event('config.changed')
@event('action.db-backup')

denote that after one pass through of the loop the event is going away? I
know there have been a lot of discussions on this already so I'm not sure
if this is a step backwards or not.

On Tue, Jun 28, 2016 at 12:22 PM Cory Johns notifications@github.com
wrote:

It just occurred to me that one of the goals of integrating actions with
reactive is to be able to gate or choose action implementation based on
state which would imply that we'd want @action to work with @When etc.
Currently, there's the outstanding bug of @When and @hook not working
together because of the phase issue. I think we might need to figure out if
we can address that if we're going to go this route.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub
#66 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAET1TmP1YHrt3nKuPiYGIIhYy5qu7Zvks5qQUo1gaJpZM4IW_qH
.

[stub42]

@marcoceppi The trick is ordering, as @hook and @action need their single invocation to be made between the setup and main phases. If @hook can be transitioned, so can @action as they are pretty much identical. There is no reason to pile it onto this PR though, as it seems to be a separate issue.

@johnsca Similarly, I'm not sure why @action needs to block on being able to mix @when decorators. Fair enough if fixing @hook is short term, as it will save some typing. But any approach to fixing @hook will also apply to @setup and @action.

[stub42]

Is there anything blocking this? The remaining issues I'm aware of of can all go into separate branches, and I see nothing that needs to be in-place before landing this PR:

• CLI integration
• #12 (Mixing @when and @hook, and now @action)
• Base Layer integration (juju-solutions/layer-basic#69)
• charm-tools integration

[stub42]

Can this please land or be rejected? Its been in limbo for over 6 months, as are the things I need this for.

[arosales]

@stub42 apologies for the delay in reply here. Chatting with @johnsca we do need to get consensus from @bcsaller and @marcoceppi on the design on actions in the reactive framework.

@bcsaller @marcoceppi @johnsca,
Could you guys comment on the design here so @stub42 can move forward with actions in reactive and/or work to find a recommended approach/work around for actions?

-thanks,
Antonio

[johnsca]

Specifically, the outstanding questions as I understood them were:

• Is it ok that this somewhat reinforces the idea of being tied to the hook context? (I seem to recall we were leaning toward this being ok for actions, as they are inherently different from hooks in that they are more on-demand.)
• This doesn't address the boillerplate necessary to load the reactive framework, as well as checking to ensure that the bootstrap has been run first. Does that matter for this or should it just be handled as an additional PR? How would you use the new decorator without it?
• Related to the previous, should the actions invoke the rest of the reactive bus? Should it only be invoked if states are changed by the action?

[stub42]

For the boilerplate, juju-solutions/layer-basic#69 adds an actions/action.template to the base layer. It can land after this PR. charm-tools will need a new feature, so charmer authors can declare actions and have 'charm build' install the template with the correct action name, but that work is not required and can be done after this PR.

Per discussions in Pasadena, I think it is critical that reactive actions invoke the rest of the reactive bus. If you don't want this, you should write a traditional action. Adding reactive action support in no way stops you writing traditional, non-reactive actions and I have use cases for both approaches.

[stub42]

Can this please land or be rejected? Its been in limbo for over 6 months, as are the things I need this for.

[arosales]

@stub42

Thanks for your patience and code on this pull. To your point on reactive actions invoking the rest of the reactive bus is the sticking point on the design of the reactive framework. I know there are strong opinions on both sides, and I think this has been the main point of the length of this pull.

Also to your point I think this pull is at a point where we should:

• land
• redesign
• close

I'll confirm with @johnsca and @marcoceppi on Tuesday (Jan 24) on the direction this pull should go and act accordingly.

-thanks,
Antonio

[johnsca]

There has been a lot of discussion on how to best model actions in reactive and there seems to be general agreement that actions are fundamentally not reactive, in that they are an explicit, user generated invocation rather than a reaction to an event which represents a change in the state of the system, such as hook. I think that the desired integration with the reactive pattern boils down to three things:

• Ensuring that the actions use the same Python interpreter and library paths
• The ability to guard an action based on reactive states
• The ability for handlers to be triggered based on state changes made in the action

The first isn't relevant to this PR. The second can be done using is_state instead of @when decorators and fits more with the imperative nature of actions. And I think that the third is better addressed through thoughtful redesign of how the bus functions to enable state changes to come from non-hook sources in a more natural way, for which I've started a discussion on #90. In the meantime, however, the final point can also be easily worked around with a one line call to invoke the reactive bus.

Therefore, I don't want to move forward with adding new primitives to the framework to support this, and I'm going to close this PR in favor of the discussion in #90.