Skip to content

Commit

Permalink
Merge pull request #131 from canonical/roll-back-relation-id-rename
Browse files Browse the repository at this point in the history
Roll back relation id rename
  • Loading branch information
PietroPasotti authored May 29, 2024
2 parents da30b05 + d30205d commit 653470b
Show file tree
Hide file tree
Showing 13 changed files with 33 additions and 23 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -471,15 +471,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 `id`.
Every time you instantiate `Relation` (or peer, or subordinate), the new instance will be given a unique `relation_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.id == next_id
assert rel.relation_id == next_id
```

This can be handy when using `replace` to create new relations, to avoid relation ID conflicts:
Expand All @@ -488,8 +488,8 @@ This can be handy when using `replace` to create new relations, to avoid relatio
import scenario.state

rel = scenario.Relation('foo')
rel2 = rel.replace(local_app_data={"foo": "bar"}, id=scenario.state.next_relation_id())
assert rel2.id == rel.id + 1
rel2 = rel.replace(local_app_data={"foo": "bar"}, relation_id=scenario.state.next_relation_id())
assert rel2.relation_id == rel.relation_id + 1
```

If you don't do this, and pass both relations into a `State`, you will trigger a consistency checker error.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta"
[project]
name = "ops-scenario"

version = "6.0.4"
version = "6.0.5"

authors = [
{ name = "Pietro Pasotti", email = "[email protected]" }
Expand Down
6 changes: 3 additions & 3 deletions scenario/consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,13 @@ def _get_relations(r):
expected_sub = relation_meta.get("scope", "") == "container"
relations = _get_relations(endpoint)
for relation in relations:
if relation.id in seen_ids:
if relation.relation_id in seen_ids:
errors.append(
f"duplicate relation ID: {relation.id} is claimed "
f"duplicate relation ID: {relation.relation_id} is claimed "
f"by multiple Relation instances",
)

seen_ids.add(relation.id)
seen_ids.add(relation.relation_id)
is_sub = isinstance(relation, SubordinateRelation)
if is_sub and not expected_sub:
errors.append(
Expand Down
6 changes: 4 additions & 2 deletions scenario/mocking.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def _get_relation_by_id(
) -> Union["Relation", "SubordinateRelation", "PeerRelation"]:
try:
return next(
filter(lambda r: r.id == rel_id, self._state.relations),
filter(lambda r: r.relation_id == rel_id, self._state.relations),
)
except StopIteration:
raise RelationNotFoundError()
Expand Down Expand Up @@ -245,7 +245,9 @@ def status_get(self, *, is_app: bool = False):

def relation_ids(self, relation_name):
return [
rel.id for rel in self._state.relations if rel.endpoint == relation_name
rel.relation_id
for rel in self._state.relations
if rel.endpoint == relation_name
]

def relation_list(self, relation_id: int) -> Tuple[str, ...]:
Expand Down
2 changes: 1 addition & 1 deletion scenario/ops_main_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,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.id # type: ignore
event.relation.relation_id # type: ignore
if event.name.endswith("_relation_broken")
else None
)
Expand Down
2 changes: 1 addition & 1 deletion scenario/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path):
env.update(
{
"JUJU_RELATION": relation.endpoint,
"JUJU_RELATION_ID": str(relation.id),
"JUJU_RELATION_ID": str(relation.relation_id),
"JUJU_REMOTE_APP": remote_app_name,
},
)
Expand Down
4 changes: 2 additions & 2 deletions scenario/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ class RelationBase(_DCBase):
"""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."""

id: int = dataclasses.field(default_factory=next_relation_id)
relation_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."""

Expand Down Expand Up @@ -1484,7 +1484,7 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent:

snapshot_data = {
"relation_name": relation.endpoint,
"relation_id": relation.id,
"relation_id": relation.relation_id,
"app_name": remote_app,
"unit_name": f"{remote_app}/{self.relation_remote_unit_id}",
}
Expand Down
12 changes: 9 additions & 3 deletions tests/test_consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,9 @@ 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", relation_id=1), Relation("bar", relation_id=1)]
),
Event("start"),
_CharmSpec(
MyCharm,
Expand All @@ -417,13 +419,17 @@ 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", relation_id=1), Relation("bar", relation_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", relation_id=1), Relation("bar", relation_id=2)]
),
Event("start"),
_CharmSpec(
MyCharm,
Expand Down
2 changes: 1 addition & 1 deletion tests/test_e2e/test_deferred.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,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.id,
"relation_id": rel.relation_id,
"app_name": "remote",
"unit_name": "remote/1",
}
Expand Down
4 changes: 2 additions & 2 deletions tests/test_e2e/test_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_ip_get(mycharm):
interface="foo",
remote_app_name="remote",
endpoint="metrics-endpoint",
id=1,
relation_id=1,
),
],
networks={"foo": Network.default(private_address="4.4.4.4")},
Expand Down Expand Up @@ -110,7 +110,7 @@ def test_no_relation_error(mycharm):
interface="foo",
remote_app_name="remote",
endpoint="metrics-endpoint",
id=1,
relation_id=1,
),
],
networks={"bar": Network.default()},
Expand Down
2 changes: 1 addition & 1 deletion tests/test_e2e/test_play_assertions.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def check_relation_data(charm):
Relation(
endpoint="relation_test",
interface="azdrubales",
id=1,
relation_id=1,
remote_app_name="karlos",
remote_app_data={"yaba": "doodle"},
remote_units_data={0: {"foo": "bar"}, 1: {"baz": "qux"}},
Expand Down
2 changes: 1 addition & 1 deletion tests/test_e2e/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def test_relation_ids():
initial_id = _next_relation_id_counter
for i in range(10):
rel = Relation("foo")
assert rel.id == initial_id + i
assert rel.relation_id == initial_id + i


def test_broken_relation_not_in_model_relations(mycharm):
Expand Down
4 changes: 3 additions & 1 deletion tests/test_e2e/test_secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,9 @@ def __init__(self, *args):
state = State(
leader=True,
relations=[
Relation("bar", remote_app_name=relation_remote_app, id=relation_id)
Relation(
"bar", remote_app_name=relation_remote_app, relation_id=relation_id
)
],
)

Expand Down

0 comments on commit 653470b

Please sign in to comment.