Fix merging.

Most particularly, the relation_id to id change was merged in main (mistakenly) and then unmerged, so the rebasing undid it, so that is redone. Also, the basic changes for Pebble notices and Cloud Configuration are done in here, although those need to be double-checked to make sure they make sense with the updated API.
canonical · Jul 2, 2024 · 66fe4a5 · 66fe4a5
1 parent e5355f4
commit 66fe4a5
Show file tree

Hide file tree

Showing 14 changed files with 39 additions and 44 deletions.
diff --git a/README.md b/README.md
@@ -439,15 +439,15 @@ needs to set up the process that will run `ops.main` with the right environment
 
 ### Working with relation IDs
 
-Every time you instantiate `Relation` (or peer, or subordinate), the new instance will be given a unique `relation_id`.
+Every time you instantiate `Relation` (or peer, or subordinate), the new instance will be given a unique `id`.
 To inspect the ID the next relation instance will have, you can call `scenario.state.next_relation_id`.
 
 ```python
 import scenario.state
 
 next_id = scenario.state.next_relation_id(update=False)
 rel = scenario.Relation('foo')
-assert rel.relation_id == next_id
+assert rel.id == next_id
 ``` 
 
 This can be handy when using `replace` to create new relations, to avoid relation ID conflicts:

diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py
@@ -125,7 +125,7 @@ def check_event_consistency(
     state: "State",
     **_kwargs,  # noqa: U101
 ) -> Results:
-    """Check the internal consistency of the Event data structure.
+    """Check the internal consistency of the _Event data structure.
 
     For example, it checks that a relation event has a relation instance, and that
     the relation endpoint name matches the event prefix.
@@ -514,13 +514,13 @@ def _get_relations(r):
         expected_sub = relation_meta.get("scope", "") == "container"
         relations = _get_relations(endpoint)
         for relation in relations:
-            if relation.relation_id in seen_ids:
+            if relation.id in seen_ids:
                 errors.append(
-                    f"duplicate relation ID: {relation.relation_id} is claimed "
+                    f"duplicate relation ID: {relation.id} is claimed "
                     f"by multiple Relation instances",
                 )
 
-            seen_ids.add(relation.relation_id)
+            seen_ids.add(relation.id)
             is_sub = isinstance(relation, SubordinateRelation)
             if is_sub and not expected_sub:
                 errors.append(
@@ -604,7 +604,7 @@ def check_containers_consistency(
 def check_cloudspec_consistency(
     *,
     state: "State",
-    event: "Event",
+    event: "_Event",
     charm_spec: "_CharmSpec",
     **_kwargs,  # noqa: U101
 ) -> Results:

diff --git a/scenario/mocking.py b/scenario/mocking.py
@@ -163,7 +163,7 @@ def _get_relation_by_id(
     ) -> Union["Relation", "SubordinateRelation", "PeerRelation"]:
         try:
             return next(
-                filter(lambda r: r.relation_id == rel_id, self._state.relations),
+                filter(lambda r: r.id == rel_id, self._state.relations),
             )
         except StopIteration:
             raise RelationNotFoundError()
@@ -245,9 +245,7 @@ def status_get(self, *, is_app: bool = False):
 
     def relation_ids(self, relation_name):
         return [
-            rel.relation_id
-            for rel in self._state.relations
-            if rel.endpoint == relation_name
+            rel.id for rel in self._state.relations if rel.endpoint == relation_name
         ]
 
     def relation_list(self, relation_id: int) -> Tuple[str, ...]:

diff --git a/scenario/ops_main_mock.py b/scenario/ops_main_mock.py
@@ -118,7 +118,7 @@ def setup_framework(
         # If we are in a RelationBroken event, we want to know which relation is
         # broken within the model, not only in the event's `.relation` attribute.
         broken_relation_id = (
-            event.relation.relation_id  # type: ignore
+            event.relation.id  # type: ignore
             if event.name.endswith("_relation_broken")
             else None
         )

diff --git a/scenario/runtime.py b/scenario/runtime.py
@@ -211,7 +211,7 @@ def _get_event_env(self, state: "State", event: "_Event", charm_root: Path):
             env.update(
                 {
                     "JUJU_RELATION": relation.endpoint,
-                    "JUJU_RELATION_ID": str(relation.relation_id),
+                    "JUJU_RELATION_ID": str(relation.id),
                     "JUJU_REMOTE_APP": remote_app_name,
                 },
             )

diff --git a/scenario/state.py b/scenario/state.py
@@ -340,7 +340,7 @@ class _RelationBase:
     """Interface name. Must match the interface name attached to this endpoint in metadata.yaml.
     If left empty, it will be automatically derived from metadata.yaml."""
 
-    relation_id: int = dataclasses.field(default_factory=next_relation_id)
+    id: int = dataclasses.field(default_factory=next_relation_id)
     """Juju relation ID. Every new Relation instance gets a unique one,
     if there's trouble, override."""
 
@@ -575,7 +575,7 @@ def next_notice_id(update=True):
 
 
 @dataclasses.dataclass(frozen=True)
-class Notice(_DCBase):
+class Notice:
     key: str
     """The notice key, a string that differentiates notices of this type.
 
@@ -634,15 +634,15 @@ def _to_ops(self) -> pebble.Notice:
 
 
 @dataclasses.dataclass(frozen=True)
-class _BoundNotice(_DCBase):
+class _BoundNotice:
     notice: Notice
     container: "Container"
 
     @property
     def event(self):
         """Sugar to generate a <container's name>-pebble-custom-notice event for this notice."""
         suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX
-        return Event(
+        return _Event(
             path=normalize_name(self.container.name) + suffix,
             container=self.container,
             notice=self.notice,
@@ -762,6 +762,7 @@ def get_notice(
             f"{self.name} does not have a notice with key {key} and type {notice_type}",
         )
 
+
 _RawStatusLiteral = Literal[
     "waiting",
     "blocked",
@@ -1367,7 +1368,7 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent:
 
             snapshot_data = {
                 "relation_name": relation.endpoint,
-                "relation_id": relation.relation_id,
+                "relation_id": relation.id,
                 "app_name": remote_app,
                 "unit_name": f"{remote_app}/{self.relation_remote_unit_id}",
             }

diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py
@@ -69,18 +69,18 @@ def test_workload_event_without_container():
     )
     assert_inconsistent(
         State(),
-        Event("foo-pebble-custom-notice", container=Container("foo")),
+        _Event("foo-pebble-custom-notice", container=Container("foo")),
         _CharmSpec(MyCharm, {}),
     )
     notice = Notice("example.com/foo")
     assert_consistent(
         State(containers=[Container("foo", notices=[notice])]),
-        Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice),
+        _Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice),
         _CharmSpec(MyCharm, {"containers": {"foo": {}}}),
     )
     assert_inconsistent(
         State(containers=[Container("foo")]),
-        Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice),
+        _Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice),
         _CharmSpec(MyCharm, {"containers": {"foo": {}}}),
     )
 
@@ -459,9 +459,7 @@ def test_action_params_type(ptype, good, bad):
 
 def test_duplicate_relation_ids():
     assert_inconsistent(
-        State(
-            relations=[Relation("foo", id=1), Relation("bar", id=1)]
-        ),
+        State(relations=[Relation("foo", id=1), Relation("bar", id=1)]),
         _Event("start"),
         _CharmSpec(
             MyCharm,
@@ -474,17 +472,13 @@ def test_duplicate_relation_ids():
 
 def test_relation_without_endpoint():
     assert_inconsistent(
-        State(
-            relations=[Relation("foo", id=1), Relation("bar", id=1)]
-        ),
+        State(relations=[Relation("foo", id=1), Relation("bar", id=1)]),
         _Event("start"),
         _CharmSpec(MyCharm, meta={"name": "charlemagne"}),
     )
 
     assert_consistent(
-        State(
-            relations=[Relation("foo", id=1), Relation("bar", id=2)]
-        ),
+        State(relations=[Relation("foo", id=1), Relation("bar", id=2)]),
         _Event("start"),
         _CharmSpec(
             MyCharm,
@@ -658,7 +652,7 @@ def test_cloudspec_consistency():
 
     assert_consistent(
         State(model=Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec)),
-        Event("start"),
+        _Event("start"),
         _CharmSpec(
             MyCharm,
             meta={"name": "MyVMCharm"},
@@ -667,7 +661,7 @@ def test_cloudspec_consistency():
 
     assert_inconsistent(
         State(model=Model(name="k8s-model", type="kubernetes", cloud_spec=cloud_spec)),
-        Event("start"),
+        _Event("start"),
         _CharmSpec(
             MyCharm,
             meta={"name": "MyK8sCharm"},

diff --git a/tests/test_e2e/test_cloud_spec.py b/tests/test_e2e/test_cloud_spec.py
@@ -47,14 +47,14 @@ def test_get_cloud_spec():
             name="lxd-model", type="lxd", cloud_spec=scenario_cloud_spec
         ),
     )
-    with ctx.manager("start", state=state) as mgr:
+    with ctx.manager(ctx.on.start(), state=state) as mgr:
         assert mgr.charm.model.get_cloud_spec() == expected_cloud_spec
 
 
 def test_get_cloud_spec_error():
     ctx = scenario.Context(MyCharm, meta={"name": "foo"})
     state = scenario.State(model=scenario.Model(name="lxd-model", type="lxd"))
-    with ctx.manager("start", state) as mgr:
+    with ctx.manager(ctx.on.start(), state) as mgr:
         with pytest.raises(ops.ModelError):
             mgr.charm.model.get_cloud_spec()
 
@@ -65,6 +65,6 @@ def test_get_cloud_spec_untrusted():
     state = scenario.State(
         model=scenario.Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec),
     )
-    with ctx.manager("start", state) as mgr:
+    with ctx.manager(ctx.on.start(), state) as mgr:
         with pytest.raises(ops.ModelError):
             mgr.charm.model.get_cloud_spec()
diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py
@@ -169,7 +169,7 @@ def test_deferred_relation_event_from_relation(mycharm):
     assert out.deferred[0].name == "foo_relation_changed"
     assert out.deferred[0].snapshot_data == {
         "relation_name": rel.endpoint,
-        "relation_id": rel.relation_id,
+        "relation_id": rel.id,
         "app_name": "remote",
         "unit_name": "remote/1",
     }

diff --git a/tests/test_e2e/test_network.py b/tests/test_e2e/test_network.py
@@ -48,7 +48,7 @@ def test_ip_get(mycharm):
                     interface="foo",
                     remote_app_name="remote",
                     endpoint="metrics-endpoint",
-                    relation_id=1,
+                    id=1,
                 ),
             ],
             networks={"foo": Network.default(private_address="4.4.4.4")},
@@ -110,7 +110,7 @@ def test_no_relation_error(mycharm):
                     interface="foo",
                     remote_app_name="remote",
                     endpoint="metrics-endpoint",
-                    relation_id=1,
+                    id=1,
                 ),
             ],
             networks={"bar": Network.default()},

diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py
@@ -1,5 +1,5 @@
-import datetime
 import dataclasses
+import datetime
 import tempfile
 from pathlib import Path
 

diff --git a/tests/test_e2e/test_play_assertions.py b/tests/test_e2e/test_play_assertions.py
@@ -104,7 +104,7 @@ def check_relation_data(charm):
             Relation(
                 endpoint="relation_test",
                 interface="azdrubales",
-                relation_id=1,
+                id=1,
                 remote_app_name="karlos",
                 remote_app_data={"yaba": "doodle"},
                 remote_units_data={0: {"foo": "bar"}, 1: {"baz": "qux"}},

diff --git a/tests/test_e2e/test_relations.py b/tests/test_e2e/test_relations.py
@@ -407,7 +407,7 @@ def test_relation_ids():
     initial_id = _next_relation_id_counter
     for i in range(10):
         rel = Relation("foo")
-        assert rel.relation_id == initial_id + i
+        assert rel.id == initial_id + i
 
 
 def test_broken_relation_not_in_model_relations(mycharm):

diff --git a/tests/test_e2e/test_secrets.py b/tests/test_e2e/test_secrets.py
@@ -195,6 +195,9 @@ def test_set_legacy_behaviour(mycharm):
         )
 
         secret.set_content(rev2)
+        # We need to get the secret again, because ops caches the content in
+        # the object.
+        secret: ops_Secret = charm.model.get_secret(label="mylabel")
         assert (
             secret.get_content()
             == secret.peek_content()
@@ -204,6 +207,7 @@ def test_set_legacy_behaviour(mycharm):
 
         secret.set_content(rev3)
         state_out = mgr.run()
+        secret: ops_Secret = charm.model.get_secret(label="mylabel")
         assert (
             secret.get_content()
             == secret.peek_content()
@@ -505,9 +509,7 @@ def __init__(self, *args):
     state = State(
         leader=True,
         relations=[
-            Relation(
-                "bar", remote_app_name=relation_remote_app, relation_id=relation_id
-            )
+            Relation("bar", remote_app_name=relation_remote_app, id=relation_id)
         ],
     )