On Wed, Feb 17, 2016 at 5:56 PM Matt Bruzek <matthew.bru...@canonical.com> wrote:
> I hate to go against the love-in here. While I desperately want a > configuration changed feature for the reactive framework. I disagree with > the way it was implemented. I brought up issues with this implementation in > the pull request: https://github.com/juju-solutions/layer-basic/pull/36 > I appreciate the feedback on the feature > It feels wrong to lump configuration change events in to states. There > should be a different way to handle these types of events that are > generated by Juju. We already have @hook('config-changed') for when the > traditional hook would be called. When a configuration changes it would be > more natural should have a different decorator for that such as: > @when_changed('key') or more importantly > @when_any_changed('key1','key2','key3') > I'd want to avoid having /another/ primitive. We start to create our own language and it becomes this DSL rather than a simple declaration of state. I actually disagree. I feel this is the perfect way to describe the situation. Under the covers the base layer determines that configuration options have changed since last hook run, and sets the state letting the charm author know "Hey, we've got new configuration, respond accordingly". We really don't want people to implement @hook call if they can avoid it as it sets up a weird precedent in processing. Hook is invoked prior to @when decorations and could lead to a mixed state. > Charm states are completely arbitrary and an author (who may not read this > list) could legitimately set a 'config.changed' state that would create > invalid changed events and break other layers. Even if we pick a different > state name it seems we are inviting a possible collision that would be very > difficult to diagnose and debug. > You're right, but this argument could be applied to /any/ state. Having a way to review layers is the best way to curb this behavior. As a result we've recommended authors to prefix their states with the layer/scope name. For example, interfaces set states based on the relation name they're responding to. The Django layer sets it's states as django.<state> > It feels strange that we are "reserving" a state name (or many names) for > the configuration changes in this way. Implementing the feature with a > different decorator seems more deterministic and not overloading the states > with configuration events. > This isn't really overloading, we're processing data and setting the state of the charm at that time. This is very similar to how the leadership layer and apt layer work that stub has created. I feel this fits well with the state of the charm. Having a feature specific to configuration means we should do the same for things like storage, actions, relations, etc which all still fit in this current model. > > Thoughts? > I understand your point of view, but I think if you look at this from a new charmer perspective, one without years of charm authoring to date, they're going to be looking to respond to states, and needing to address configuration changes is a state much like all other states in charm (layer.restart as an example is an event modeled as state). Marco
-- Juju mailing list Juju@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju
