diff --git a/CHANGES.md b/CHANGES.md index ffe365b35..6fd7d6c78 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,7 @@ # 2.11.0 * `StopEvent`, `RemoveEvent`, and all `LifeCycleEvent`s are no longer deferrable, and will raise a `RuntimeError` if `defer()` is called on the event object. +* The remote app name (and its databag) is now consistently available in relation-broken events. * Added `ActionEvent.id`, exposing the JUJU_ACTION_UUID environment variable. * Added support for creating `pebble.Plan` objects by passing in a `pebble.PlanDict`, the ability to compare two `Plan` objects with `==`, and the ability to create an empty Plan with `Plan()`. diff --git a/ops/model.py b/ops/model.py index 4fdc39deb..cf1f065d2 100644 --- a/ops/model.py +++ b/ops/model.py @@ -1550,15 +1550,6 @@ def __iter__(self): return iter(self._data) def __getitem__(self, key: Union['Unit', 'Application']): - if key is None and self.relation.app is None: - # NOTE: if juju gets fixed to set JUJU_REMOTE_APP for relation-broken events, then that - # should fix the only case in which we expect key to be None - potentially removing the - # need for this error in future ops versions (i.e. if relation.app is guaranteed to not - # be None. See https://bugs.launchpad.net/juju/+bug/1960934. - raise KeyError( - 'Cannot index relation data with "None".' - ' Are you trying to access remote app data during a relation-broken event?' - ' This is not allowed.') return self._data[key] def __repr__(self): diff --git a/ops/testing.py b/ops/testing.py index 4eba4f96d..5014ef516 100644 --- a/ops/testing.py +++ b/ops/testing.py @@ -2167,21 +2167,9 @@ def relation_remote_app_name(self, relation_id: int) -> Optional[str]: if relation_id not in self._relation_app_and_units: # Non-existent or dead relation return None - if 'relation_broken' in self._hook_is_running: - # TODO: if juju ever starts setting JUJU_REMOTE_APP in relation-broken hooks runs, - # then we should kill this if clause. - # See https://bugs.launchpad.net/juju/+bug/1960934 - return None return self._relation_app_and_units[relation_id]['app'] def relation_get(self, relation_id: int, member_name: str, is_app: bool): - if 'relation_broken' in self._hook_is_running and not self.relation_remote_app_name( - relation_id) and member_name != self.app_name and member_name != self.unit_name: - # TODO: if juju gets fixed to set JUJU_REMOTE_APP for this case, then we may opt to - # allow charms to read/get that (stale) relation data. - # See https://bugs.launchpad.net/juju/+bug/1960934 - raise RuntimeError( - 'remote-side relation data cannot be accessed during a relation-broken event') if is_app and '/' in member_name: member_name = member_name.split('/')[0] if relation_id not in self._relation_data_raw: diff --git a/test/test_testing.py b/test/test_testing.py index 0e5db2214..47dd36e16 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -33,7 +33,6 @@ import uuid from unittest.mock import MagicMock, patch -import pytest import yaml import ops @@ -344,24 +343,6 @@ def _on_cluster_relation_changed(self, event: ops.EventBase): self.assertTrue(len(harness.charm.observed_events), 1) self.assertIsInstance(harness.charm.observed_events[0], ops.RelationEvent) - def test_relation_get_when_broken(self): - harness = ops.testing.Harness(RelationBrokenTester, meta=''' - name: test-app - requires: - foo: - interface: foofoo - ''') - self.addCleanup(harness.cleanup) - harness.begin() - harness.charm.observe_relation_events('foo') - - # relation remote app is None to mirror production Juju behavior where Juju doesn't - # communicate the remote app to ops. - rel_id = harness.add_relation('foo', None) # type: ignore - - with pytest.raises(KeyError, match='trying to access remote app data'): - harness.remove_relation(rel_id) - def test_remove_relation(self): harness = ops.testing.Harness(RelationEventCharm, meta=''' name: test-app @@ -3234,14 +3215,6 @@ def _observe_relation_event(self, event_name: str, event: ops.RelationEvent): self.changes.append(recording) -class RelationBrokenTester(RelationEventCharm): - """Access inaccessible relation data.""" - - def _on_relation_broken(self, event: ops.RelationBrokenEvent): - # We expect this to fail, because the relation has broken. - event.relation.data[event.relation.app]['bar'] # type: ignore - - class ContainerEventCharm(RecordingCharm): """Record events related to container lifecycles."""