Skip to content

Commit

Permalink
Merge pull request #113 from tonyandrewmeyer/rename-relation-id
Browse files Browse the repository at this point in the history
refactor!: rename `Relation.relation_id` to `Relation.id`
  • Loading branch information
PietroPasotti authored May 22, 2024
2 parents 97b50f8 + 354050b commit e758e78
Show file tree
Hide file tree
Showing 12 changed files with 22 additions and 32 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 `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:
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"}, relation_id=scenario.state.next_relation_id())
assert rel2.relation_id == rel.relation_id + 1
rel2 = rel.replace(local_app_data={"foo": "bar"}, id=scenario.state.next_relation_id())
assert rel2.id == rel.id + 1
```

If you don't do this, and pass both relations into a `State`, you will trigger a consistency checker error.
Expand Down
6 changes: 3 additions & 3 deletions scenario/consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,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(
Expand Down
6 changes: 2 additions & 4 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.relation_id == rel_id, self._state.relations),
filter(lambda r: r.id == rel_id, self._state.relations),
)
except StopIteration:
raise RelationNotFoundError()
Expand Down Expand Up @@ -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, ...]:
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.relation_id # type: ignore
event.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.relation_id),
"JUJU_RELATION_ID": str(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 @@ -353,7 +353,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."""

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."""

Expand Down Expand Up @@ -1410,7 +1410,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}",
}
Expand Down
12 changes: 3 additions & 9 deletions tests/test_consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,7 @@ def test_action_params_type(ptype, good, bad):

def test_duplicate_relation_ids():
assert_inconsistent(
State(
relations=[Relation("foo", relation_id=1), Relation("bar", relation_id=1)]
),
State(relations=[Relation("foo", id=1), Relation("bar", id=1)]),
Event("start"),
_CharmSpec(
MyCharm,
Expand All @@ -416,17 +414,13 @@ def test_duplicate_relation_ids():

def test_relation_without_endpoint():
assert_inconsistent(
State(
relations=[Relation("foo", relation_id=1), Relation("bar", relation_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", relation_id=1), Relation("bar", relation_id=2)]
),
State(relations=[Relation("foo", id=1), Relation("bar", 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.relation_id,
"relation_id": rel.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",
relation_id=1,
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",
relation_id=1,
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",
relation_id=1,
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.relation_id == initial_id + i
assert rel.id == initial_id + i


def test_broken_relation_not_in_model_relations(mycharm):
Expand Down
4 changes: 1 addition & 3 deletions tests/test_e2e/test_secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,9 +521,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)
],
)

Expand Down

0 comments on commit e758e78

Please sign in to comment.