<div dir="ltr">These sound like good steps to me. I would just encourage us to look at the worst examples of charms people have written or questions that are common and attempt to defend against that sort of thing. People invent things out of perceived necessity and if we can find those and provide solutions I think we'll have a better product in the end.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Oct 4, 2017 at 9:00 AM Marco Ceppi <<a href="mailto:marco.ceppi@canonical.com">marco.ceppi@canonical.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">So, I've not actually checked the logs in a while, but if visibility is an issue, it seems reasonable that the reactive framework should print in the log that X flags are being reset due to hook failure. Things like set_flag_immediate has farther reaching consequences than simply stating that flags only get set on success of a hook run.<div><br></div><div>I know there are further reaching initiatives to alleviate this by decoupling the reactive state engine from the juju hooks system. In this case each successive loop of the reactive runtime could better snapshot state and make failures more granular.</div><div><br></div><div>* state is being renamed to flag in the next major version of reactive.<br><br><div class="gmail_quote"></div></div></div><div dir="ltr"><div><div class="gmail_quote"><div dir="ltr">On Wed, Oct 4, 2017 at 8:52 AM Mike Wilson <<a href="mailto:mike.wilson@canonical.com" target="_blank">mike.wilson@canonical.com</a>> wrote:<br></div></div></div></div><div dir="ltr"><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">So as a new charm writer coming to Juju I would first do this:<div><div style="color:rgb(33,33,33)"><br></div><div style="color:rgb(33,33,33)">def get_ready():<br></div><div style="color:rgb(33,33,33)">    func0()</div><div style="color:rgb(33,33,33)">    func1_fails()</div><div style="color:rgb(33,33,33)"><br></div><div style="color:rgb(33,33,33)">Then I would, hopefully, test and notice issues. I would investigate and see that I needed to be idempotent. My next attempt would be to wrap those functions inside state checks with sets after they complete. This would also fail and now the charm creator is left with nothing in the api that can help. They are now off to their own devices to start doing random things to attempt to make this work the way they want it to work. Hopefully, the solution is as straight-forward as touching random files, but we just never know.</div><div style="color:rgb(33,33,33)"><br></div><div style="color:rgb(33,33,33)">I would expect the name of set_state to be something like set_state_on_success and I would further expect some sort of immediate state thing like set_state or set_state_immediate. This would give the user the tools we know that they need in order to create bug-free charms.</div><div style="color:rgb(33,33,33)"><br></div><div style="color:rgb(33,33,33)">Now to compound that confusion, we have the issue of a hook can call multiple functions inside the charm code and if any of those functions have something that fails the whole thing is unwrapped. I understand from a Juju perspective why this is the case, but as a user, I would be completely taken by surprise here. The only real fix here is documentation so that we can set expectations, but people will most likely look at examples instead of documentation. This means that we need to make sure to call out any potential issues like this in the example charms we release.</div><div style="color:rgb(33,33,33)"><div><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Oct 4, 2017 at 6:34 AM Stuart Bishop <<a href="mailto:stuart.bishop@canonical.com" target="_blank">stuart.bishop@canonical.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 4 October 2017 at 00:51, Mike Wilson <<a href="mailto:mike.wilson@canonical.com" target="_blank">mike.wilson@canonical.com</a>> wrote:<br>
> So the best practice here is to touch a file and test for the existence of<br>
> that file before running must_be_called_exactly_once()?<br>
><br>
> I think part of the issue here is that without knowing the extent of the<br>
> hook it is hard to enforce idempotency as a charm writer. It's easy to look<br>
> at the code above and say that is it idempotent since the init function is<br>
> wrapped in a when_not and the initialized state is set at the bottom of<br>
> init.<br>
<br>
Individual handlers should be idempotent, so it doesn't matter about<br>
the extent of the hook, or even if the chained handlers being triggers<br>
are running in the same hook. Assume your handlers get called multiple<br>
times, because they may be. Yes, it looks idempotent but it isn't. An<br>
assumption is being made that the state changes get committed<br>
immediately, but these changes are actually transactional and<br>
following the same transactional behaviour as the Juju hook<br>
environment [1]. I think this can certainly be explained better in the<br>
docs, but I can't think of a way to stop this being an easy error to<br>
make.<br>
<br>
[1] spot the DBA<br>
<br>
--<br>
Stuart Bishop <<a href="mailto:stuart.bishop@canonical.com" target="_blank">stuart.bishop@canonical.com</a>><br>
</blockquote></div></blockquote></div></div></div><div dir="ltr"><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
--</blockquote></div></div></div><div dir="ltr"><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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>