Skip to content

Commit

Permalink
fix: the other argument to RelatationDataContent.update(...) shou…
Browse files Browse the repository at this point in the history
…ld be optional (#1226)

Fixes the signature for `RelationDataContent.update` to match
`MutableMapping`, where `other` is optional (a regression introduced in
#1883).

The type for `other` has been simplified to `Any`. It should really be
`Mapping|_SupportsKeysAndGetItem[str,str]` plus a minimal type that
supports `.values`, but it was already messy pulling in
`_SupportsKeysAndGetItem` in #1183, and we're just passing this through
to `MutableMapping` so it doesn't seem like the tight typing is
providing enough benefit to justify the complexity of the signature.
[typeshed has three
overloads](https://github.com/python/typeshed/blob/f7c03486ee01c8ea74823db75e017341bf3c2ad0/stdlib/typing.pyi#L726),
so we could match that (as we did in #1883, just incompletely), if that
is desirable.

Fixes: #1225

---------

Co-authored-by: Tony Meyer <tony.meyer@canonical.com>
  • Loading branch information
addyess and tonyandrewmeyer committed May 24, 2024
1 parent 7e7a18b commit 0dd27df
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
12 changes: 1 addition & 11 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,6 @@
})


# Copied from typeshed.
_KT = typing.TypeVar("_KT")
_VT_co = typing.TypeVar("_VT_co", covariant=True)


class _SupportsKeysAndGetItem(typing.Protocol[_KT, _VT_co]):
def keys(self) -> typing.Iterable[_KT]: ...
def __getitem__(self, __key: _KT) -> _VT_co: ...


logger = logging.getLogger(__name__)

MAX_LOG_LINE_LEN = 131071 # Max length of strings to pass to subshell.
Expand Down Expand Up @@ -1722,7 +1712,7 @@ def __getitem__(self, key: str) -> str:
self._validate_read()
return super().__getitem__(key)

def update(self, other: _SupportsKeysAndGetItem[str, str], **kwargs: str):
def update(self, other: typing.Any = (), /, **kwargs: str):
"""Update the data from dict/iterable other and the kwargs."""
super().update(other, **kwargs)

Expand Down
26 changes: 26 additions & 0 deletions test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,32 @@ def test_get_app_relation_data(self, harness: ops.testing.Harness[ops.CharmBase]
relation_id, harness.model.app) == harness.get_relation_data(
relation_id, local_app) == {'foo': 'bar'}

@pytest.mark.parametrize('args,kwargs', [
(({'foo': 'baz'}, ), {}),
(([('foo', 'baz')], ), {}),
((), {'foo': 'baz'})
])
def test_update_app_relation_data(
self,
args: typing.Tuple[typing.Any, ...],
kwargs: typing.Dict[str, str],
harness: ops.testing.Harness[ops.CharmBase],
):
harness.set_leader(True)
harness.begin()
relation_id = harness.add_relation('db1', 'remote')
harness.add_relation_unit(relation_id, 'remote/0')
with harness._event_context('foo_event'):
harness.update_relation_data(
relation_id,
harness.model.app.name,
{'foo': 'bar'})
rel = harness.model.get_relation('db1', relation_id)
assert rel is not None
rel.data[harness.model.app].update(*args, **kwargs)
assert harness.get_relation_data(
relation_id, harness.model.app) == {'foo': 'baz'}

def test_unit_relation_data(self, harness: ops.testing.Harness[ops.CharmBase]):
relation_id = harness.add_relation('db1', 'remoteapp1')
harness.add_relation_unit(relation_id, 'remoteapp1/0')
Expand Down

0 comments on commit 0dd27df

Please sign in to comment.