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

removed remote unit ids var from Relation #63

Merged
merged 4 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,6 @@ To declare a peer relation, you should use `scenario.state.PeerRelation`. The co
that peer relations do not have a "remote app" (it's this app, in fact). So unlike `Relation`, a `PeerRelation` does not
have `remote_app_name` or `remote_app_data` arguments. Also, it talks in terms of `peers`:

- `Relation.remote_unit_ids` maps to `PeerRelation.peers_ids`
- `Relation.remote_units_data` maps to `PeerRelation.peers_data`

```python
Expand Down Expand Up @@ -488,7 +487,7 @@ remote unit that the event is about. The reason that this parameter is not suppl
events, is that the relation already ties 'this app' to some 'remote app' (cfr. the `Relation.remote_app_name` attr),
but not to a specific unit. What remote unit this event is about is not a `State` concern but an `Event` one.

The `remote_unit_id` will default to the first ID found in the relation's `remote_unit_ids`, but if the test you are
The `remote_unit_id` will default to the first ID found in the relation's `remote_units_data`, but if the test you are
writing is close to that domain, you should probably override it and pass it manually.

```python
Expand Down
4 changes: 3 additions & 1 deletion scenario/mocking.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ def relation_list(self, relation_id: int) -> Tuple[str]:
relation = self._get_relation_by_id(relation_id)

if isinstance(relation, PeerRelation):
return tuple(f"{self.app_name}/{unit_id}" for unit_id in relation.peers_ids)
return tuple(
f"{self.app_name}/{unit_id}" for unit_id in relation.peers_data
)
return tuple(
f"{relation.remote_app_name}/{unit_id}"
for unit_id in relation._remote_unit_ids
Expand Down
80 changes: 8 additions & 72 deletions scenario/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,74 +318,18 @@ def broken_event(self) -> "Event":
)


def unify_ids_and_remote_units_data(ids: List[int], data: Dict[int, Any]):
"""Unify and validate a list of unit IDs and a mapping from said ids to databag contents.

This allows the user to pass equivalently:
ids = []
data = {1: {}}

or

ids = [1]
data = {}

or

ids = [1]
data = {1: {}}

but catch the inconsistent:

ids = [1]
data = {2: {}}

or

ids = [2]
data = {1: {}}
"""
if ids and data:
if not set(ids) == set(data):
raise StateValidationError(
f"{ids} should include any and all IDs from {data}",
)
elif ids:
data = {x: {} for x in ids}
elif data:
ids = list(data)
else:
ids = [0]
data = {0: {}}
return ids, data


@dataclasses.dataclass(frozen=True)
class Relation(RelationBase):
remote_app_name: str = "remote"

# fixme: simplify API by deriving remote_unit_ids from remote_units_data.
remote_unit_ids: List[int] = dataclasses.field(default_factory=list)

# local limit
limit: int = 1

remote_app_data: Dict[str, str] = dataclasses.field(default_factory=dict)
remote_units_data: Dict[int, Dict[str, str]] = dataclasses.field(
default_factory=dict,
default_factory=lambda: {0: {}},
)

def __post_init__(self):
super().__post_init__()

remote_unit_ids, remote_units_data = unify_ids_and_remote_units_data(
self.remote_unit_ids,
self.remote_units_data,
)
# bypass frozen dataclass
object.__setattr__(self, "remote_unit_ids", remote_unit_ids)
object.__setattr__(self, "remote_units_data", remote_units_data)

@property
def _remote_app_name(self) -> str:
"""Who is on the other end of this relation?"""
Expand All @@ -394,7 +338,7 @@ def _remote_app_name(self) -> str:
@property
def _remote_unit_ids(self) -> Tuple[int]:
"""Ids of the units on the other end of this relation."""
return tuple(self.remote_unit_ids)
return tuple(self.remote_units_data)

def _get_databag_for_remote(self, unit_id: int) -> Dict[str, str]:
"""Return the databag for some remote unit ID."""
Expand Down Expand Up @@ -447,10 +391,11 @@ def remote_unit_name(self) -> str:

@dataclasses.dataclass(frozen=True)
class PeerRelation(RelationBase):
peers_data: Dict[int, Dict[str, str]] = dataclasses.field(default_factory=dict)

# IDs of the peers. Consistency checks will validate that *this unit*'s ID is not in here.
peers_ids: List[int] = dataclasses.field(default_factory=list)
peers_data: Dict[int, Dict[str, str]] = dataclasses.field(
PietroPasotti marked this conversation as resolved.
Show resolved Hide resolved
default_factory=lambda: {0: {}},
)
# mapping from peer unit IDs to their databag contents.
# Consistency checks will validate that *this unit*'s ID is not in here.

@property
def _databags(self):
Expand All @@ -462,21 +407,12 @@ def _databags(self):
@property
def _remote_unit_ids(self) -> Tuple[int]:
"""Ids of the units on the other end of this relation."""
return tuple(self.peers_ids)
return tuple(self.peers_data)

def _get_databag_for_remote(self, unit_id: int) -> Dict[str, str]:
"""Return the databag for some remote unit ID."""
return self.peers_data[unit_id]

def __post_init__(self):
peers_ids, peers_data = unify_ids_and_remote_units_data(
self.peers_ids,
self.peers_data,
)
# bypass frozen dataclass guards
object.__setattr__(self, "peers_ids", peers_ids)
object.__setattr__(self, "peers_data", peers_data)


def _random_model_name():
import random
Expand Down
5 changes: 1 addition & 4 deletions tests/test_e2e/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ def pre_event(charm: CharmBase):
interface="bar",
local_app_data={"a": "because"},
remote_app_name="remote",
remote_unit_ids=[0, 1, 2],
remote_app_data={"a": "b"},
local_unit_data={"c": "d"},
remote_units_data={0: {}, 1: {"e": "f"}, 2: {}},
Expand Down Expand Up @@ -198,9 +197,7 @@ def pre_event(charm: CharmBase):
endpoint="foo",
interface="bar",
remote_app_name="remote",
remote_unit_ids=[1, 4],
local_app_data={},
local_unit_data={},
remote_units_data={1: {}, 4: {}},
)
state = State(
leader=True,
Expand Down