Skip to content

Commit

Permalink
fix: use StatusBase.__init_subclass__ instead of decorator to avoid o…
Browse files Browse the repository at this point in the history
…bscuring subclass type (canonical#1383)

Previously, StatusBase.register obscured the type of decorated classes.
The classes would all end up typed as Type[StatusBase], and when instantiated
they would be typed as StatusBase instead of their actual class (see canonical#1380).
Instead we now register subclasses automatically in __init_subclass__,
no longer using StatusBase.register as a decorator, which avoids
obscuring the type of subclasses.
  • Loading branch information
james-garner-canonical authored Sep 22, 2024
1 parent 454e197 commit 20c9623
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 9 deletions.
9 changes: 3 additions & 6 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1907,6 +1907,9 @@ def __init__(self, message: str = ''):
raise TypeError('cannot instantiate a base class')
self.message = message

def __init_subclass__(cls):
StatusBase.register(cls)

def __eq__(self, other: 'StatusBase') -> bool:
if not isinstance(self, type(other)):
return False
Expand Down Expand Up @@ -1965,7 +1968,6 @@ def _get_highest_priority(cls, statuses: 'List[StatusBase]') -> 'StatusBase':
return max(statuses, key=lambda status: cls._priorities.get(status.name, 0))


@StatusBase.register
class UnknownStatus(StatusBase):
"""The unit status is unknown.
Expand All @@ -1986,7 +1988,6 @@ def __repr__(self):
return 'UnknownStatus()'


@StatusBase.register
class ErrorStatus(StatusBase):
"""The unit status is error.
Expand All @@ -2000,7 +2001,6 @@ class ErrorStatus(StatusBase):
name = 'error'


@StatusBase.register
class ActiveStatus(StatusBase):
"""The unit or application is ready and active.
Expand All @@ -2016,7 +2016,6 @@ def __init__(self, message: str = ''):
super().__init__(message)


@StatusBase.register
class BlockedStatus(StatusBase):
"""The unit or application requires manual intervention.
Expand All @@ -2027,7 +2026,6 @@ class BlockedStatus(StatusBase):
name = 'blocked'


@StatusBase.register
class MaintenanceStatus(StatusBase):
"""The unit or application is performing maintenance tasks.
Expand All @@ -2041,7 +2039,6 @@ class MaintenanceStatus(StatusBase):
name = 'maintenance'


@StatusBase.register
class WaitingStatus(StatusBase):
"""The unit or application is waiting on a charm it's integrated with.
Expand Down
10 changes: 7 additions & 3 deletions test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -820,11 +820,15 @@ def test_base_status_instance_raises(self):
with pytest.raises(TypeError):
ops.StatusBase('test')

class NoNameStatus(ops.StatusBase):
pass
with pytest.raises(TypeError):
# TypeError due to missing `name` attribute
class NoNameStatus(ops.StatusBase): # pyright: ignore[reportUnusedClass]
pass

with pytest.raises(TypeError):
ops.StatusBase.register(NoNameStatus)
# TypeError due to non str type `name` attribute
class NonStringNameStatus(ops.StatusBase): # pyright: ignore[reportUnusedClass]
name = None # pyright: ignore[reportAssignmentType]

def test_status_repr(self):
test_cases = {
Expand Down
3 changes: 3 additions & 0 deletions test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -3232,6 +3232,9 @@ def test_get_filesystem_root(self):

def test_evaluate_status(self):
class TestCharm(ops.CharmBase):
app_status_to_add: ops.StatusBase
unit_status_to_add: ops.StatusBase

def __init__(self, framework: ops.Framework):
super().__init__(framework)
self.framework.observe(self.on.collect_app_status, self._on_collect_app_status)
Expand Down

0 comments on commit 20c9623

Please sign in to comment.