<div dir="ltr">I'll have to check with Jorge Castro, but I imagine either the 13th or 27th of this month. I'll confirm and this will likely be a "Europe" friendly time.<div><br></div><div>Marco</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, May 2, 2016 at 5:19 PM Merlijn Sebrechts <<a href="mailto:merlijn.sebrechts@gmail.com">merlijn.sebrechts@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Great suggestion, Marco! When would the next office hour be?</div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-02 23:13 GMT+02:00 Marco Ceppi <span dir="ltr"><<a href="mailto:marco.ceppi@canonical.com" target="_blank">marco.ceppi@canonical.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">Might I suggest we do a hangout on air so we can record the discussion while skipping the back and forth on the list? Possibly during an office hour?</p> <p dir="ltr">Also, I'm not sure the decision is final and I certainly appreciate your feedback and welcome the continued discussion so we can reach a consensus!</p><span><font color="#888888"> <p dir="ltr">Marco</p></font></span><div><div> <br><div class="gmail_quote"><div dir="ltr">On Mon, May 2, 2016, 4:09 PM Merlijn Sebrechts <<a href="mailto:merlijn.sebrechts@gmail.com" target="_blank">merlijn.sebrechts@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Cory<div><br></div><div><br></div><div>Thanks for your consideration. I strongly agree that any sort of automatic state removal is a bad idea. That was the reason why I started thinking about making the differentiation between states and events. I would have loved to discuss this more thoroughly with you and Ben. Although I understand the decision has been made, I would still like to explain my take on this, especially since we agree on so much of the fundamentals.</div><div><br></div><div>Each state is a fact. A fact can only become un-true when an action reverses it. x.installed will becomes untrue when something uninstalls x. If you interpret y.changed as a fact, then it will only become untrue when y has reverted to its original value. Only then does it become un-changed. This behavior is clearly useless. So in contrary to all the other states, "x.changed" was not interpreted as a fact. It has been interpreted as "x.changed since the last hook run" by removing this state after a hook run.</div><div><br></div><div>I am glad that we agree that this behavior isn't consistent and that it has to change. Now I'm not so sure about the fix. Removing the "x.changed" hook manually in a handler has the exact same issue. "x.changed" has not been made un-true because some handler reacts to it. "x.changed" is still a fact. By removing it, the handlers are actually lying to the framework. This will cause all sorts of issues.</div><div><br></div><div>Am I correct that you will modify the reactive framework to not retest the queue on a state removal? I understand the reasoning behind it, however, this will create new issues. Retesting the queue ensures a hook run has the same outcome no matter what order the handlers are executed in. A handler should not be allowed to run when its conditions aren't satisfied anymore. Please see the following example:</div><div><br></div><div>Handler A requires the service to be running. Handler B stops the service.</div><div><br></div><div>When the queue is A-B, you will have a successful run. When the queue is B-A, you will have an error. The order in which handlers are executed is not determined, so this means that <b>this hook would crash sometimes, and run successfully other times</b>. This will cause errors that are not reproducible. Reproducability and repeatability are very important in config management...<br></div><div><br></div><div>I would love to discuss this more thoroughly with you and Ben. Doing a discussion like this on a mailinglist isn't the easiest way of communicating, although I'm not sure the time difference permits a real-time discussion.</div><div><br></div><div><br></div><div><br></div><div>Kind regards</div></div><div dir="ltr"><div>Merlijn Sebrechts</div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-02 21:15 GMT+02:00 Cory Johns <span dir="ltr"><<a href="mailto:cory.johns@canonical.com" target="_blank">cory.johns@canonical.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Merlijn,<div><br></div><div>Apologies for the delayed reply. I realized that I had typed this up but forgotten to actually send it.<br><div><br></div><div>You're right that there are still cases where the hook-persistent nature of the config.changed states continue to cause problems. However, after some discussion with Ben, I actually think that *any* sort of automatic state removal is the wrong approach, whether it happens at the end of a hook or at the end of an dispatch loop (essentially what you're proposing with events). Instead, Ben convinced me that the right thing to do is to always have states be explicitly acknowledged and removed by the handlers. This doesn't work as expected currently because of an implementation detail of how the handler queue is managed on state removals, but I think it's more appropriate to fix that rather than add a new type of state.<br></div><div><br></div><div>In that approach, the config.changed state would be set when the change is detected, all applicable handlers that are watching for it would execute, each one explicitly acknowledging that it's been handled by removing it, and then, after all handlers are done, the removals would be applied. Note that the initial handler (e.g., install or start_service) would also need to clear the changed state if present to prevent the secondary handler (reinstall or restart_service) from acting on it. Alternatively, the approach I took for the ibm-base layer was to remove the gating state and separate reinstall handlers entirely, and always just drive off the config.changed states: <a href="https://code.launchpad.net/~johnsca/layer-ibm-base/fix-multi-call/+merge/292845" target="_blank">https://code.launchpad.net/~johnsca/layer-ibm-base/fix-multi-call/+merge/292845</a></div></div><div><br></div><div>Note that there is still some potential semantic value to having "new" and "changed" be distinguishable, but perhaps it's not as valuable enough to worry about.</div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 22, 2016 at 6:57 PM, Merlijn Sebrechts <span dir="ltr"><<a href="mailto:merlijn.sebrechts@gmail.com" target="_blank">merlijn.sebrechts@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Cory<div><br></div><div><br></div><div><br></div><div>I think this is another symptom of the deeper issue that the reactive framework treats events like states. 'config.changed' is an event. The following code is something that intuitively seems correct. We want to reinstall when the config has changed while the service is installed. However, it will still have the unwanted side effect you stated earlier. </div><div><br></div><div><pre style="white-space:pre-wrap;overflow:auto;font-family:Consolas,'Liberation Mono',Menlo,Courier,monospace;font-size:11.9px;margin-top:0px;margin-bottom:0px;font-stretch:normal;line-height:1.45;padding:16px;border-radius:3px;word-wrap:normal;color:rgb(51,51,51);background-color:rgb(247,247,247)"><span style="color:rgb(121,93,163)">@when</span>('installed', <span style="color:rgb(24,54,145)">'config.changed.install_source'</span>) <span style="color:rgb(167,29,93)">def</span> <span style="color:rgb(121,93,163)">reinstall</span>(): install()</pre></div><div class="gmail_extra"><br></div><div class="gmail_extra">Please note that <b>your fix will only work when the service is installed during the first config-changed hook</b>. If a service is installed during a subsequent config-changed hook, you will again have the same issue. This can happen when you have config options such as "(bool)install_plugin-x" and "(string)plugin-x-source".</div><div class="gmail_extra"><br></div><div class="gmail_extra">Anticipating these kind of conflicts requires a thorough understanding of both the reactive framework and hooks. You are correct in thinking that these conflicts should not happen. If we require every Charmer to have full understanding of these things, we might miss out on valuable contributions.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">I would urge you to seriously consider making the differentiation between events and states. For people who have used hooks it might seem logical that config.changed is active during an entire hook. Newcomers might have more difficulty understanding this.<br></div><div class="gmail_extra"><div class="gmail_quote"><br></div><div class="gmail_quote">So my suggestion is:</div><div class="gmail_quote"><br></div><div class="gmail_quote"> - An event is only active right after the event happens.</div><div class="gmail_quote"> - A handler can only be added to the queue when his events + his states are active</div><div class="gmail_quote"> - A handler will be removed from the queue only when one of his states becomes inactive. Events of handlers that are in the queue are not 'rechecked'.</div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote">Another use-case for this:</div><div class="gmail_quote"><br></div><div class="gmail_quote">@when('service.running', 'configfile.changed')</div><div class="gmail_quote">def restart_service()</div><div class="gmail_quote"><br></div><div class="gmail_quote"> 1. When the config file changes, and the service is running, restart the service.</div><div class="gmail_quote"> 2. When the config file changes and the service is not running, don't restart the service.</div><div class="gmail_quote"> 3. When the config file changed before the service was running, and now we start the service, don't restart the service.</div><div class="gmail_quote"> 4. When the config file changes, the service restarts, and the config file changes again, we want to restart the service again.</div><div class="gmail_quote"><br></div><div class="gmail_quote">1 and 2 are currently possible. 3 and 4 would be if 'file.changed' would be an event.</div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote">Kind regards</div><div class="gmail_quote">Merlijn Sebrechts</div><div class="gmail_quote"><br></div><div class="gmail_quote"><div><div>2016-04-22 23:02 GMT+02:00 Cory Johns <span dir="ltr"><<a href="mailto:cory.johns@canonical.com" target="_blank">cory.johns@canonical.com</a>></span>:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div><div dir="ltr">I have proposed <a href="https://github.com/juju-solutions/layer-basic/pull/61" target="_blank">https://github.com/juju-solutions/layer-basic/pull/61</a> as a slight change to how the config.changed states from the basic layer work. Currently, the changed states are set during the first hook invocation, under the assumption that the values were "changed" from "nothing" (not being set at all). However, this is slightly problematic in a case like the following, where we expect install() to only be called once, unless the value has changed after the fact:<div><br></div><div><pre style="overflow:auto;font-family:Consolas,'Liberation Mono',Menlo,Courier,monospace;font-size:11.9px;margin-top:0px;margin-bottom:0px;font-stretch:normal;line-height:1.45;padding:16px;border-radius:3px;word-wrap:normal;color:rgb(51,51,51);background-color:rgb(247,247,247)"><span style="color:rgb(121,93,163)">@when_not</span>(<span style="color:rgb(24,54,145)"><span>'</span>installed<span>'</span></span>) <span style="color:rgb(167,29,93)">def</span> <span style="color:rgb(121,93,163)">install</span>(): <span style="color:rgb(150,152,150)"># do install</span> set_state(<span style="color:rgb(24,54,145)"><span>'</span>installed<span>'</span></span>) <span style="color:rgb(121,93,163)">@when</span>(<span style="color:rgb(24,54,145)"><span>'</span>config.changed.install_source<span>'</span></span>) <span style="color:rgb(167,29,93)">def</span> <span style="color:rgb(121,93,163)">reinstall</span>(): install()</pre></div><div><br></div><div>The proposal adds new states, config.new, and changes config.changed to not be set the first time. You could get the old behavior by saying @when_any('config.new.foo', 'config.changed.foo').</div><div><br></div><div>Is anyone depending on the current behavior? Are there any objections to this change?</div></div> <br></div></div><span><font color="#888888">--<br> Juju mailing list<br> <a href="mailto:Juju@lists.ubuntu.com" target="_blank">Juju@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju</a><br> <br></font></span></blockquote></div><br></div></div> </blockquote></div><br></div> </div></div></blockquote></div><br></div> --<br> Juju mailing list<br> <a href="mailto:Juju@lists.ubuntu.com" target="_blank">Juju@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju</a><br> </blockquote></div> </div></div></blockquote></div><br></div> </blockquote></div>