Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some events should not be deferrable #740

Closed
pengale opened this issue Apr 21, 2022 · 5 comments
Closed

Some events should not be deferrable #740

pengale opened this issue Apr 21, 2022 · 5 comments
Assignees
Labels
feature New feature or request small item

Comments

@pengale
Copy link
Contributor

pengale commented Apr 21, 2022

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.

@jameinel
Copy link
Member

jameinel commented Apr 21, 2022 via email

@pengale
Copy link
Contributor Author

pengale commented Apr 22, 2022

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 :-)

@pengale pengale added the feature New feature or request label Sep 2, 2022
@benhoyt
Copy link
Collaborator

benhoyt commented May 1, 2023

Events like ActionEvent.defer raise RuntimeError for this, so we could follow that precedent for these "tearing down" events.

@benhoyt
Copy link
Collaborator

benhoyt commented Oct 4, 2023

This is still an issue. However, we're thinking of (maybe!) deprecating defer in general, see #966

@tonyandrewmeyer
Copy link
Contributor

This was done in #1122, and I missed linking the PR to the ticket, sorry 😞.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request small item
Projects
None yet
Development

No branches or pull requests

4 participants