Some events should not be deferrable
When Juju is tearing down a unit, deferring does not work as one might naively expect. Since the controller does not have a view of "deferred" or "non deferred" events, the controller will simply assume that the "deferred" hook finished, and will continue the teardown.
When charm authors call event.defer on events like storage-detached, and possibly relation-departed and relation-broken, the framework should raise an error.
Error states are visible in the controller. This means that the code will do what the charm author probably "intended", which is to block the teardown until something else happens.
We already have code that actions cannot be deferred, we can probably use the same errors in this situation.
On Thu, Apr 21, 2022 at 12:13 PM Pen Gale @.***> wrote:
When Juju is tearing down a unit, deferring does not work as one might naively expect. Since the controller does not have a view of "deferred" or "non deferred" events, the controller will simply assume that the "deferred" hook finished, and will continue the teardown.
When charm authors call event.defer on events like storage-detached, and possibly relation-departed and relation-broken, the framework should raise an error.
Error states are visible in the controller. This means that the code will do what the charm author probably "intended", which is to block the teardown until something else happens.
— Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/740, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7NYBCK7UBLQEP6KL73VGF5BDANCNFSM5T7XVM2Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>
We already have code that actions cannot be deferred, we can probably use the same errors in this situation.
Excellent. Writing less code is always good :-)
Events like ActionEvent.defer raise RuntimeError for this, so we could follow that precedent for these "tearing down" events.
This is still an issue. However, we're thinking of (maybe!) deprecating defer in general, see https://github.com/canonical/operator/issues/966
This was done in #1122, and I missed linking the PR to the ticket, sorry :disappointed:.