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.

Sign up for GitHub

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

Jump to bottom

feat: make it an error to call CollectStatusEvent.add_status with error or unknown #1386

Merged
merged 4 commits into from
Sep 25, 2024
Merged

feat: make it an error to call CollectStatusEvent.add_status with error or unknown #1386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ff7f13
Make it an error to call CollectStatusEvent.add_status with error or …
james-garner-canonical Sep 23, 2024
aeaa732
Remove overly verbose type anotations and just keep runtime checks
james-garner-canonical Sep 23, 2024
e3edf3f
Document exceptions using Raises
james-garner-canonical Sep 25, 2024
06fefe1
Revert "Document exceptions using Raises"
james-garner-canonical Sep 25, 2024
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
9 changes: 7 additions & 2 deletions ops/charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,12 +1105,13 @@ class CollectStatusEvent(LifecycleEvent):

The order of priorities is as follows, from highest to lowest:

* error
* blocked
* maintenance
* waiting
* active
* unknown
james-garner-canonical marked this conversation as resolved.
Show resolved Hide resolved

It is an error to call :meth:`add_status` with an instance of
:class:`ErrorStatus` or :class:`UnknownStatus`.
james-garner-canonical marked this conversation as resolved.
Show resolved Hide resolved

If there are multiple statuses with the same priority, the first one added
wins (and if an event is observed multiple times, the handlers are called
Expand Down Expand Up @@ -1151,6 +1152,10 @@ def add_status(self, status: model.StatusBase):
"""
if not isinstance(status, model.StatusBase):
raise TypeError(f'status should be a StatusBase, not {type(status).__name__}')
if status.name not in model._SETTABLE_STATUS_NAMES:
raise model.InvalidStatusError(
f'status.name must be in {model._SETTABLE_STATUS_NAMES}, not {status.name!r}'
)
model_ = self.framework.model
if self.handle.kind == 'collect_app_status':
if not isinstance(status, model.ActiveStatus):
Expand Down
4 changes: 2 additions & 2 deletions test/test_charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,6 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
(['waiting', 'blocked'], 'blocked'),
(['waiting', 'maintenance'], 'maintenance'),
(['active', 'waiting'], 'waiting'),
(['active', 'unknown'], 'active'),
],
)
def test_collect_status_priority_valid(
Expand Down Expand Up @@ -994,6 +993,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
[
['blocked', 'error'],
['unknown'],
['active', 'unknown'],
],
)
def test_collect_status_priority_invalid(
Expand All @@ -1015,7 +1015,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):

framework = create_framework(request)
charm = MyCharm(framework, statuses=statuses)
with pytest.raises(ModelError):
with pytest.raises(ops.InvalidStatusError):
ops.charm._evaluate_status(charm)


Expand Down
Loading