-
Notifications
You must be signed in to change notification settings - Fork 119
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
Run deferred Juju events on a fresh charm instance #1174
Comments
I haven't thought about this specific case a ton, but I suspect there's a number of charms in the wild that have bugs around this and don't realize it. I know at least some charms have |
Per Madrid discussion: it still seems to us like a good idea to run deferred events on their own charm instance so they're more like other events. However, we don't want to spend time on this until we've worked through the "Juju-aware deferral" mechanism we're planning for 24.10. |
The main benefit here would be that it would be simpler to reason about (Juju) event behaviour, because it would be consistent both when an event was deferred and when it is executed normally.
This would not apply to custom events, which are executed synchronously, and are already different from Juju events. This would also not apply to
LifecycleEvents
(framework generated events), because those cannot be deferred.We will need to use the same charm instance for multiple deferred (event, handler) pairs when more than one handler defers the event. If something like this happens:
I don't think we can tell that these were different Juju events, and we would maybe collapse the duplicates (?) and probably have to use the same charm instance for both observers. We would otherwise need to add some sort of ID to the deferred event to be able to group them correctly.
I believe the cleanest change here is to lift out the deferred
reemit
from_Manager
in [ops/main.py], and handle the deferred events inmain()
, creating new_Managers
for each one (we need to check thatsetup_root_logging
can be called twice, and avoid calling legacy hooks twice - probably the legacy hook should be moved out of_Manager
too). This provides a freshModel
,Framework
and_Dispatcher
as well, which is also most consistent (and avoids having to have the framework forget/unobserve the old charm).There would be some small behaviour changes here: the pre-commit and commit events would get emitted twice (just as if the event had not been deferred, but not like now), which I think is fine, and we would collect the status twice (again just as if the event had not been deferred, but not like now), which might break some expectations.
See also the discussion in #736. Note that we will not be able to make Harness mimic this behaviour (because it would break too many existing charm tests) but we will be encouraging using Scenario, which can handle it (right now Scenario copies the ops behaviour, saving the deferred events to a queue that ops then runs ahead of the Juju event, but the Scenario API would not need changing in order to match the new ops behaviour).
The text was updated successfully, but these errors were encountered: