Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roll back relation id rename #131

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading