Repository navigation
fix: don't drop deferred custom events in ops.testing - #2811
Draft
tonyandrewmeyer wants to merge 2 commits into
Draft
tonyandrewmeyer wants to merge 2 commits into
tonyandrewmeyer wants to merge 2 commits into
Conversation
State.deferred only picked up notices whose event snapshot matched ops'
event regex, which needs an 'on/' in the handle path. A custom event from
an ObjectEvents created with a key ('MyCharm/on[key]/event[n]') or from an
EventSource directly on the charm ('MyCharm/event[n]') doesn't have one,
so the deferral was silently dropped and the event never ran again, while
ops itself re-emits every notice. The same snapshot then showed up in
State.stored_states as a bogus StoredState.
Read every notice instead, and leave anything with a notice out of the
stored states.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NfZieBLjUD8cULjqRQpHm
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
State.deferredonly picks up a notice if its event's handle path matches ops' event regex, which needs anon/. A deferred custom event from anObjectEventscreated with a key (MyCharm/on[key]/event[n]), or from anEventSourcedirectly on the charm (MyCharm/event[n]), is silently dropped, so its handler never runs again, although ops itself re-emits every notice whatever the path. The same snapshot also shows up inState.stored_statesas a bogusStoredState.This reads every notice instead, as ops does, and leaves anything with a notice out of the stored states.
Hyrum has one newly failing test and none newly passing. Valkey's
test_start_primarydefersunit_fully_started(anEventSourceon itsBaseEventsobject) in onestartrun and asserts the status after the next. With this change the deferred event is re-emitted before thatstart, as it would be in Juju, so the status differs. I think that's the test relying on the bug rather than a reason to keep it, so I think we should make this change, but open a PR to fix things there.It turned up in a multi-charm test of Valkey, where the dropped event meant its start lock was never released.