From de6cede44635b55f8def5237f2db77874bf2409b Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Tue, 3 Sep 2024 16:03:54 +1200
Subject: [PATCH 01/17] Fix broken test of registering StatusBase subclass

Previously was catching AttributeError, which must have been raised
historically for missing `name` attribute on StatusBase subclasses.
This, combined with a `type: ignore` directive concealed that the
`register_status` method does not exist -- the attempt to access it
raises an AttributeError. The correct method is `register`, which
raises a TypeError.
---
 test/test_model.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test_model.py b/test/test_model.py
index c8266dc90..197573467 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -823,8 +823,8 @@ def test_base_status_instance_raises(self):
         class NoNameStatus(ops.StatusBase):
             pass
 
-        with pytest.raises(AttributeError):
-            ops.StatusBase.register_status(NoNameStatus)  # type: ignore
+        with pytest.raises(TypeError):
+            ops.StatusBase.register(NoNameStatus)
 
     def test_status_repr(self):
         test_cases = {

From b58602771720f6e99ae2562943d5e0de7f68135b Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Tue, 3 Sep 2024 16:10:26 +1200
Subject: [PATCH 02/17] Fix broken status_set test

A `type: ignore` directive hid that `status_set` was being called with
the wrong argument types. The intention was to test that an error is
raised on calling with a non-bool argument for `is_app`, however the
status argument should be a string, and was instead a `StatusBase`
class. The test would have still correctly errored out on the `is_app`
argument before this error had an effect at testing time.
---
 test/test_model.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/test/test_model.py b/test/test_model.py
index 197573467..fd3277612 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2951,7 +2951,10 @@ def test_local_get_status(self, fake_script: FakeScript, name: str):
     def test_status_set_is_app_not_bool_raises(self):
         for is_app_v in [None, 1, 2.0, 'a', b'beef', object]:
             with pytest.raises(TypeError):
-                self.backend.status_set(ops.ActiveStatus, is_app=is_app_v)  # type: ignore
+                self.backend.status_set(
+                    'active',
+                    is_app=is_app_v,  # type: ignore[assignment]
+                )
 
     def test_storage_tool_errors(self, fake_script: FakeScript):
         fake_script.write('storage-list', 'echo fooerror >&2 ; exit 1')

From c50dd47eb7849c433b7afef9b4e3afee3b07cee4 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Tue, 3 Sep 2024 16:47:10 +1200
Subject: [PATCH 03/17] Expose settable statuses in type hint of set_status

_ModelBackend.status_set's status argument was previously type hinted
only as str. However, this argument can really only be one of four valid
statuses that juju's set-status will accept. This commit documents this,
and adjusts the definition of StatusBase to also type hint the six
acceptable status names, as well as using these type hints across code
that interacts with StatusBase and status_set.
---
 ops/model.py       | 35 +++++++++++++++++++++++++----------
 ops/testing.py     |  7 ++++---
 test/test_charm.py |  6 +++---
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index 9102f30d0..e7f12b17f 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -41,6 +41,7 @@
     Generator,
     Iterable,
     List,
+    Literal,
     Mapping,
     MutableMapping,
     Optional,
@@ -68,7 +69,10 @@
 _StorageDictType = Dict[str, Optional[List['Storage']]]
 _BindingDictType = Dict[Union[str, 'Relation'], 'Binding']
 
-_StatusDict = TypedDict('_StatusDict', {'status': str, 'message': str})
+_SettableStatusName = Literal['active', 'blocked', 'maintenance', 'waiting']
+_StatusName = Union[_SettableStatusName, Literal['error', 'unknown']]
+_StatusDict = TypedDict('_StatusDict', {'status': _StatusName, 'message': str})
+
 
 # mapping from relation name to a list of relation objects
 _RelationMapping_Raw = Dict[str, Optional[List['Relation']]]
@@ -425,7 +429,12 @@ def status(self, value: 'StatusBase'):
 
         for _key in {'name', 'message'}:
             assert isinstance(getattr(value, _key), str), f'status.{_key} must be a string'
-        self._backend.status_set(value.name, value.message, is_app=True)
+        self._backend.status_set(
+            typing.cast(_SettableStatusName, value.name),  # TODO: narrower type for value
+            value.message,
+            is_app=True,
+        )
+
         self._status = value
 
     def planned_units(self) -> int:
@@ -590,7 +599,11 @@ def status(self, value: 'StatusBase'):
             raise RuntimeError(f'cannot set status for a remote unit {self}')
 
         # fixme: if value.messages
-        self._backend.status_set(value.name, value.message, is_app=False)
+        self._backend.status_set(
+            typing.cast(_SettableStatusName, value.name),  # TODO: narrower type for value
+            value.message,
+            is_app=False,
+        )
         self._status = value
 
     def __repr__(self):
@@ -1866,10 +1879,10 @@ class StatusBase:
     directly use the child class such as :class:`ActiveStatus` to indicate their status.
     """
 
-    _statuses: Dict[str, Type['StatusBase']] = {}
+    _statuses: Dict[_StatusName, Type['StatusBase']] = {}
 
-    # Subclasses should override this attribute
-    name = ''
+    # Subclasses must provide this attribute
+    name: _StatusName
 
     def __init__(self, message: str = ''):
         if self.__class__ is StatusBase:
@@ -1885,7 +1898,7 @@ def __repr__(self):
         return f'{self.__class__.__name__}({self.message!r})'
 
     @classmethod
-    def from_name(cls, name: str, message: str):
+    def from_name(cls, name: _StatusName, message: str):
         """Create a status instance from a name and message.
 
         If ``name`` is "unknown", ``message`` is ignored, because unknown status
@@ -1907,7 +1920,7 @@ def from_name(cls, name: str, message: str):
     @classmethod
     def register(cls, child: Type['StatusBase']):
         """Register a Status for the child's name."""
-        if not isinstance(child.name, str):
+        if not (hasattr(child, 'name') and isinstance(child.name, str)):
             raise TypeError(
                 f"Can't register StatusBase subclass {child}: ",
                 'missing required `name: str` class attribute',
@@ -3396,13 +3409,15 @@ def status_get(self, *, is_app: bool = False) -> '_StatusDict':
         #       status-data: {}
 
         if is_app:
-            content = typing.cast(Dict[str, Dict[str, str]], content)
+            content = typing.cast(Dict[str, '_StatusDict'], content)
             app_status = content['application-status']
             return {'status': app_status['status'], 'message': app_status['message']}
         else:
             return typing.cast('_StatusDict', content)
 
-    def status_set(self, status: str, message: str = '', *, is_app: bool = False) -> None:
+    def status_set(
+        self, status: _SettableStatusName, message: str = '', *, is_app: bool = False
+    ) -> None:
         """Set a status of a unit or an application.
 
         Args:
diff --git a/ops/testing.py b/ops/testing.py
index 40d60fc4f..27bfc0b52 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -60,7 +60,7 @@
 from ops._private import yaml
 from ops.charm import CharmBase, CharmMeta, RelationRole
 from ops.jujucontext import _JujuContext
-from ops.model import Container, RelationNotFoundError, _NetworkDict
+from ops.model import Container, RelationNotFoundError, _NetworkDict, _StatusName
 from ops.pebble import ExecProcess
 
 ReadableBuffer = Union[bytes, str, StringIO, BytesIO, BinaryIO]
@@ -79,7 +79,6 @@
 
 _RelationEntities = TypedDict('_RelationEntities', {'app': str, 'units': List[str]})
 
-_StatusName = Literal['unknown', 'blocked', 'active', 'maintenance', 'waiting']
 _RawStatus = TypedDict(
     '_RawStatus',
     {
@@ -2489,7 +2488,9 @@ def status_get(self, *, is_app: bool = False):
         else:
             return self._unit_status
 
-    def status_set(self, status: '_StatusName', message: str = '', *, is_app: bool = False):
+    def status_set(
+        self, status: model._SettableStatusName, message: str = '', *, is_app: bool = False
+    ):
         if status in [model.ErrorStatus.name, model.UnknownStatus.name]:
             raise model.ModelError(
                 f'ERROR invalid status "{status}", expected one of'
diff --git a/test/test_charm.py b/test/test_charm.py
index b6879b151..f4562a8bc 100644
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -22,7 +22,7 @@
 
 import ops
 import ops.charm
-from ops.model import ModelError
+from ops.model import ModelError, _StatusName
 
 from .test_helpers import FakeScript, create_framework
 
@@ -967,11 +967,11 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
 def test_collect_status_priority(
     request: pytest.FixtureRequest,
     fake_script: FakeScript,
-    statuses: typing.List[str],
+    statuses: typing.List[_StatusName],
     expected: str,
 ):
     class MyCharm(ops.CharmBase):
-        def __init__(self, framework: ops.Framework, statuses: typing.List[str]):
+        def __init__(self, framework: ops.Framework, statuses: typing.List[_StatusName]):
             super().__init__(framework)
             self.framework.observe(self.on.collect_app_status, self._on_collect_status)
             self.statuses = statuses

From f8e1596c97a061e50d68a61f28df5ae50e496f45 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Tue, 3 Sep 2024 17:07:07 +1200
Subject: [PATCH 04/17] Validate status in status_set before calling status-set

Since we know the valid arguments for status, and expose them in type
hints, and since ops already validates arguments here to some extent
(is_app being a bool in status_set, status and message being strings in
Application.status and Unit.status), let's check that status is valid
in status_set and raise a ModelError immediately if not, rather than
catching status-set's exit code and only then raising a ModelError.
---
 ops/model.py       |  4 ++++
 ops/testing.py     |  2 +-
 test/test_charm.py | 34 +++++++++++++++++++++++++++++++---
 test/test_model.py | 10 ++--------
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index e7f12b17f..44c9a1bf0 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -51,6 +51,7 @@
     Type,
     TypedDict,
     Union,
+    get_args,
 )
 
 import ops
@@ -70,6 +71,7 @@
 _BindingDictType = Dict[Union[str, 'Relation'], 'Binding']
 
 _SettableStatusName = Literal['active', 'blocked', 'maintenance', 'waiting']
+_SettableStatusNames: Tuple[_SettableStatusName, ...] = get_args(_SettableStatusName)
 _StatusName = Union[_SettableStatusName, Literal['error', 'unknown']]
 _StatusDict = TypedDict('_StatusDict', {'status': _StatusName, 'message': str})
 
@@ -3428,6 +3430,8 @@ def status_set(
         """
         if not isinstance(is_app, bool):
             raise TypeError('is_app parameter must be boolean')
+        if status not in _SettableStatusNames:
+            raise ModelError(f'status must be in {_SettableStatusNames}, not {status}')
         self._run('status-set', f'--application={is_app}', status, message)
 
     def storage_list(self, name: str) -> List[int]:
diff --git a/ops/testing.py b/ops/testing.py
index 27bfc0b52..98b37d85d 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -2491,7 +2491,7 @@ def status_get(self, *, is_app: bool = False):
     def status_set(
         self, status: model._SettableStatusName, message: str = '', *, is_app: bool = False
     ):
-        if status in [model.ErrorStatus.name, model.UnknownStatus.name]:
+        if status not in model._SettableStatusNames:
             raise model.ModelError(
                 f'ERROR invalid status "{status}", expected one of'
                 ' [maintenance blocked waiting active]'
diff --git a/test/test_charm.py b/test/test_charm.py
index f4562a8bc..2f399a25b 100644
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -956,15 +956,13 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
 @pytest.mark.parametrize(
     'statuses,expected',
     [
-        (['blocked', 'error'], 'error'),
         (['waiting', 'blocked'], 'blocked'),
         (['waiting', 'maintenance'], 'maintenance'),
         (['active', 'waiting'], 'waiting'),
         (['active', 'unknown'], 'active'),
-        (['unknown'], 'unknown'),
     ],
 )
-def test_collect_status_priority(
+def test_collect_status_priority_valid(
     request: pytest.FixtureRequest,
     fake_script: FakeScript,
     statuses: typing.List[_StatusName],
@@ -991,6 +989,36 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
     assert status_set_calls == [['status-set', '--application=True', expected, '']]
 
 
+@pytest.mark.parametrize(
+    'statuses',
+    [
+        ['blocked', 'error'],
+        ['unknown'],
+    ],
+)
+def test_collect_status_priority_invalid(
+    request: pytest.FixtureRequest,
+    fake_script: FakeScript,
+    statuses: typing.List[_StatusName],
+):
+    class MyCharm(ops.CharmBase):
+        def __init__(self, framework: ops.Framework, statuses: typing.List[_StatusName]):
+            super().__init__(framework)
+            self.framework.observe(self.on.collect_app_status, self._on_collect_status)
+            self.statuses = statuses
+
+        def _on_collect_status(self, event: ops.CollectStatusEvent):
+            for status in self.statuses:
+                event.add_status(ops.StatusBase.from_name(status, ''))
+
+    fake_script.write('is-leader', 'echo true')
+
+    framework = create_framework(request)
+    charm = MyCharm(framework, statuses=statuses)
+    with pytest.raises(ModelError):
+        ops.charm._evaluate_status(charm)
+
+
 def test_meta_links():
     # Each type of link can be a single item.
     meta = ops.CharmMeta.from_yaml("""
diff --git a/test/test_model.py b/test/test_model.py
index fd3277612..86f25b632 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2878,12 +2878,11 @@ def test_status_is_app_forced_kwargs(self, fake_script: FakeScript):
                 case()
 
     def test_local_set_invalid_status(self, fake_script: FakeScript):
-        # juju returns exit code 1 if you ask to set status to 'unknown' or 'error'
+        # ops will directly raise a ModelError if you try to set status to 'unknown' or 'error'
         meta = ops.CharmMeta.from_yaml("""
             name: myapp
         """)
         model = ops.Model(meta, self.backend)
-        fake_script.write('status-set', 'exit 1')
         fake_script.write('is-leader', 'echo true')
 
         with pytest.raises(ops.ModelError):
@@ -2891,10 +2890,7 @@ def test_local_set_invalid_status(self, fake_script: FakeScript):
         with pytest.raises(ops.ModelError):
             model.unit.status = ops.ErrorStatus()
 
-        assert fake_script.calls(True) == [
-            ['status-set', '--application=False', 'unknown', ''],
-            ['status-set', '--application=False', 'error', ''],
-        ]
+        assert fake_script.calls(True) == []
 
         with pytest.raises(ops.ModelError):
             model.app.status = ops.UnknownStatus()
@@ -2904,8 +2900,6 @@ def test_local_set_invalid_status(self, fake_script: FakeScript):
         # A leadership check is needed for application status.
         assert fake_script.calls(True) == [
             ['is-leader', '--format=json'],
-            ['status-set', '--application=True', 'unknown', ''],
-            ['status-set', '--application=True', 'error', ''],
         ]
 
     @pytest.mark.parametrize('name', ['active', 'waiting', 'blocked', 'maintenance', 'error'])

From f9423de0df1dc59e909a25039c013eaa6de69fd2 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Tue, 3 Sep 2024 17:19:19 +1200
Subject: [PATCH 05/17] Validate type of message in set_status

Since _ModelBackend.set_status validates its status and is_app
arguments, let's also validate the type of message, removing redundant
checks from Application.status and cleaning up a fixme in Unit.status.
Also add a test to cover this, analogous to the existing test for the
type of is_app.
---
 ops/model.py       | 5 ++---
 test/test_model.py | 8 ++++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index 44c9a1bf0..efa484eeb 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -429,8 +429,6 @@ def status(self, value: 'StatusBase'):
         if not self._backend.is_leader():
             raise RuntimeError('cannot set application status as a non-leader unit')
 
-        for _key in {'name', 'message'}:
-            assert isinstance(getattr(value, _key), str), f'status.{_key} must be a string'
         self._backend.status_set(
             typing.cast(_SettableStatusName, value.name),  # TODO: narrower type for value
             value.message,
@@ -600,7 +598,6 @@ def status(self, value: 'StatusBase'):
         if not self._is_our_unit:
             raise RuntimeError(f'cannot set status for a remote unit {self}')
 
-        # fixme: if value.messages
         self._backend.status_set(
             typing.cast(_SettableStatusName, value.name),  # TODO: narrower type for value
             value.message,
@@ -3430,6 +3427,8 @@ def status_set(
         """
         if not isinstance(is_app, bool):
             raise TypeError('is_app parameter must be boolean')
+        if not isinstance(message, str):
+            raise TypeError('message parameter must be a string')
         if status not in _SettableStatusNames:
             raise ModelError(f'status must be in {_SettableStatusNames}, not {status}')
         self._run('status-set', f'--application={is_app}', status, message)
diff --git a/test/test_model.py b/test/test_model.py
index 86f25b632..a563831c0 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2950,6 +2950,14 @@ def test_status_set_is_app_not_bool_raises(self):
                     is_app=is_app_v,  # type: ignore[assignment]
                 )
 
+    def test_status_set_message_not_str_raises(self):
+        for message in [None, 1, 2.0, True, b'beef', object]:
+            with pytest.raises(TypeError):
+                self.backend.status_set(
+                    'active',
+                    message=message,  # type: ignore[assignment]
+                )
+
     def test_storage_tool_errors(self, fake_script: FakeScript):
         fake_script.write('storage-list', 'echo fooerror >&2 ; exit 1')
         with pytest.raises(ops.ModelError):

From 424b74dee81ca558023c874a50aeae4120b45aca Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Tue, 3 Sep 2024 18:37:04 +1200
Subject: [PATCH 06/17] Revert change to signature of StatusBase.from_name

Since StatusBase.from_name is part of the public API, and is currently
used in several charms, changing the type of the name argument from str
to _StatusName will break users' tests. Furthermore, satisfying the type
checker as to the contents of the strings used may be painful for users,
leading to a proliferation of casts or `type: ignore`s. Therefore, the
type or the name argument has been reverted to str.
---
 ops/model.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index efa484eeb..9e2f51758 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -75,7 +75,6 @@
 _StatusName = Union[_SettableStatusName, Literal['error', 'unknown']]
 _StatusDict = TypedDict('_StatusDict', {'status': _StatusName, 'message': str})
 
-
 # mapping from relation name to a list of relation objects
 _RelationMapping_Raw = Dict[str, Optional[List['Relation']]]
 # mapping from container name to container metadata
@@ -1897,7 +1896,7 @@ def __repr__(self):
         return f'{self.__class__.__name__}({self.message!r})'
 
     @classmethod
-    def from_name(cls, name: _StatusName, message: str):
+    def from_name(cls, name: str, message: str):
         """Create a status instance from a name and message.
 
         If ``name`` is "unknown", ``message`` is ignored, because unknown status
@@ -1914,7 +1913,7 @@ def from_name(cls, name: _StatusName, message: str):
             # unknown is special
             return UnknownStatus()
         else:
-            return cls._statuses[name](message)
+            return cls._statuses[typing.cast(_StatusName, name)](message)
 
     @classmethod
     def register(cls, child: Type['StatusBase']):

From c295028e1d758826f51a519f934a1757fb3ea26e Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 4 Sep 2024 15:57:26 +1200
Subject: [PATCH 07/17] Remove TODOs

Unclear how to narrow type of value given that type checking doesn't
excellently support properties with differing types for getter and
setter, so sticking with a simple `cast` for now.
---
 ops/model.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index 9e2f51758..6a7d33eb6 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -429,7 +429,7 @@ def status(self, value: 'StatusBase'):
             raise RuntimeError('cannot set application status as a non-leader unit')
 
         self._backend.status_set(
-            typing.cast(_SettableStatusName, value.name),  # TODO: narrower type for value
+            typing.cast(_SettableStatusName, value.name),  # status_set will validate at runtime
             value.message,
             is_app=True,
         )
@@ -598,7 +598,7 @@ def status(self, value: 'StatusBase'):
             raise RuntimeError(f'cannot set status for a remote unit {self}')
 
         self._backend.status_set(
-            typing.cast(_SettableStatusName, value.name),  # TODO: narrower type for value
+            typing.cast(_SettableStatusName, value.name),  # status_set will validate at runtime
             value.message,
             is_app=False,
         )

From ebe61a13ddf4fe677fd79b431b0ad2ec12c1c549 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 4 Sep 2024 16:03:49 +1200
Subject: [PATCH 08/17] Change object to object() in test cases

Either work, as we're just testing that the type not being bool or str
raises an error, but bare object may raise eyebrows for readers.
---
 test/test_model.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test_model.py b/test/test_model.py
index a563831c0..81e2d15aa 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2943,7 +2943,7 @@ def test_local_get_status(self, fake_script: FakeScript, name: str):
         assert model.app.status.message == 'bar'
 
     def test_status_set_is_app_not_bool_raises(self):
-        for is_app_v in [None, 1, 2.0, 'a', b'beef', object]:
+        for is_app_v in [None, 1, 2.0, 'a', b'beef', object()]:
             with pytest.raises(TypeError):
                 self.backend.status_set(
                     'active',
@@ -2951,7 +2951,7 @@ def test_status_set_is_app_not_bool_raises(self):
                 )
 
     def test_status_set_message_not_str_raises(self):
-        for message in [None, 1, 2.0, True, b'beef', object]:
+        for message in [None, 1, 2.0, True, b'beef', object()]:
             with pytest.raises(TypeError):
                 self.backend.status_set(
                     'active',

From 3ad845992bbc70f0b486d34abe21b8deb5bc26c2 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 4 Sep 2024 16:23:18 +1200
Subject: [PATCH 09/17] Raise InvalidStatusError instead of ModelError in
 status_set

InvalidStatusError is a subclass of ModelError, so it shouldn't break
any user code, and anyway it doesn't look like people were setting
invalid statuses since Juju would give them an error at runtime
currently.
---
 ops/model.py       |  6 +++---
 ops/testing.py     |  5 ++---
 test/test_model.py | 10 +++++-----
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index 6a7d33eb6..75695fefe 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -1952,7 +1952,7 @@ class UnknownStatus(StatusBase):
     charm has not called status-set yet.
 
     This status is read-only; trying to set unit or application status to
-    ``UnknownStatus`` will raise :class:`ModelError`.
+    ``UnknownStatus`` will raise :class:`InvalidStatusError`.
     """
 
     name = 'unknown'
@@ -1973,7 +1973,7 @@ class ErrorStatus(StatusBase):
     human intervention in order to operate correctly).
 
     This status is read-only; trying to set unit or application status to
-    ``ErrorStatus`` will raise :class:`ModelError`.
+    ``ErrorStatus`` will raise :class:`InvalidStatusError`.
     """
 
     name = 'error'
@@ -3429,7 +3429,7 @@ def status_set(
         if not isinstance(message, str):
             raise TypeError('message parameter must be a string')
         if status not in _SettableStatusNames:
-            raise ModelError(f'status must be in {_SettableStatusNames}, not {status}')
+            raise InvalidStatusError(f'status must be in {_SettableStatusNames}, not {status!r}')
         self._run('status-set', f'--application={is_app}', status, message)
 
     def storage_list(self, name: str) -> List[int]:
diff --git a/ops/testing.py b/ops/testing.py
index 98b37d85d..ff920afe1 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -2492,9 +2492,8 @@ def status_set(
         self, status: model._SettableStatusName, message: str = '', *, is_app: bool = False
     ):
         if status not in model._SettableStatusNames:
-            raise model.ModelError(
-                f'ERROR invalid status "{status}", expected one of'
-                ' [maintenance blocked waiting active]'
+            raise model.InvalidStatusError(
+                f'status must be in {model._SettableStatusNames}, not {status!r}'
             )
         if is_app:
             self._app_status = {'status': status, 'message': message}
diff --git a/test/test_model.py b/test/test_model.py
index 81e2d15aa..1d3268bb5 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2878,23 +2878,23 @@ def test_status_is_app_forced_kwargs(self, fake_script: FakeScript):
                 case()
 
     def test_local_set_invalid_status(self, fake_script: FakeScript):
-        # ops will directly raise a ModelError if you try to set status to 'unknown' or 'error'
+        # ops will directly raise InvalidStatusError if you try to set status to unknown or error
         meta = ops.CharmMeta.from_yaml("""
             name: myapp
         """)
         model = ops.Model(meta, self.backend)
         fake_script.write('is-leader', 'echo true')
 
-        with pytest.raises(ops.ModelError):
+        with pytest.raises(ops.InvalidStatusError):
             model.unit.status = ops.UnknownStatus()
-        with pytest.raises(ops.ModelError):
+        with pytest.raises(ops.InvalidStatusError):
             model.unit.status = ops.ErrorStatus()
 
         assert fake_script.calls(True) == []
 
-        with pytest.raises(ops.ModelError):
+        with pytest.raises(ops.InvalidStatusError):
             model.app.status = ops.UnknownStatus()
-        with pytest.raises(ops.ModelError):
+        with pytest.raises(ops.InvalidStatusError):
             model.app.status = ops.ErrorStatus()
 
         # A leadership check is needed for application status.

From 0132af5559a5dac44540330c63179bed3e01c403 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Mon, 9 Sep 2024 14:07:29 +1200
Subject: [PATCH 10/17] List valid names in StatusBase.from_name docstring

---
 ops/model.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ops/model.py b/ops/model.py
index 75695fefe..eaef1337f 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -1903,7 +1903,8 @@ def from_name(cls, name: str, message: str):
         does not have an associated message.
 
         Args:
-            name: Name of the status, for example "active" or "blocked".
+            name: Name of the status, one of:
+                "active", "blocked", "maintenance", "waiting", "error", or "unknown".
             message: Message to include with the status.
 
         Raises:

From a92737e29186d051aa53c0714e011df5c1dcff54 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 09:08:39 +1200
Subject: [PATCH 11/17] Document InvalidStatusError cases in add_status

Calling add_status with an ErrorStatus *will* eventually result in
an InvalidStatusError. Calling add_status with an UnknownStatus *may*
eventually result in an InvalidStatusError.
---
 ops/charm.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ops/charm.py b/ops/charm.py
index e6d35c3a3..77d80e2cb 100644
--- a/ops/charm.py
+++ b/ops/charm.py
@@ -1078,12 +1078,17 @@ class CollectStatusEvent(LifecycleEvent):
 
     The order of priorities is as follows, from highest to lowest:
 
-    * error
+    * error (Note: will always result in an InvalidStatusError!)
     * blocked
     * maintenance
     * waiting
     * active
-    * unknown
+    * unknown (Note: will result in InvalidStatusError if set as the status)
+
+    Note that "error" and "unknown" are read-only statuses, and an InvalidStatusError
+    will be raised if either of these are chosen as the highest-priority status and
+    set as the status. Note also that as "error" has the highest priority, it will
+    always cause in an InvalidStatusError.
 
     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

From 8e1702091b5f94846774614902ad5d4cd27c281d Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 09:16:30 +1200
Subject: [PATCH 12/17] Use UPPER_SNAKE to better distinguish global from
 TypeAlias

---
 ops/model.py   | 6 +++---
 ops/testing.py | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index eaef1337f..0f92d7871 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -71,9 +71,9 @@
 _BindingDictType = Dict[Union[str, 'Relation'], 'Binding']
 
 _SettableStatusName = Literal['active', 'blocked', 'maintenance', 'waiting']
-_SettableStatusNames: Tuple[_SettableStatusName, ...] = get_args(_SettableStatusName)
 _StatusName = Union[_SettableStatusName, Literal['error', 'unknown']]
 _StatusDict = TypedDict('_StatusDict', {'status': _StatusName, 'message': str})
+_SETTABLE_STATUS_NAMES: Tuple[_SettableStatusName, ...] = get_args(_SettableStatusName)
 
 # mapping from relation name to a list of relation objects
 _RelationMapping_Raw = Dict[str, Optional[List['Relation']]]
@@ -3429,8 +3429,8 @@ def status_set(
             raise TypeError('is_app parameter must be boolean')
         if not isinstance(message, str):
             raise TypeError('message parameter must be a string')
-        if status not in _SettableStatusNames:
-            raise InvalidStatusError(f'status must be in {_SettableStatusNames}, not {status!r}')
+        if status not in _SETTABLE_STATUS_NAMES:
+            raise InvalidStatusError(f'status must be in {_SETTABLE_STATUS_NAMES}, not {status!r}')
         self._run('status-set', f'--application={is_app}', status, message)
 
     def storage_list(self, name: str) -> List[int]:
diff --git a/ops/testing.py b/ops/testing.py
index ff920afe1..a35a83186 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -2491,9 +2491,9 @@ def status_get(self, *, is_app: bool = False):
     def status_set(
         self, status: model._SettableStatusName, message: str = '', *, is_app: bool = False
     ):
-        if status not in model._SettableStatusNames:
+        if status not in model._SETTABLE_STATUS_NAMES:
             raise model.InvalidStatusError(
-                f'status must be in {model._SettableStatusNames}, not {status!r}'
+                f'status must be in {model._SETTABLE_STATUS_NAMES}, not {status!r}'
             )
         if is_app:
             self._app_status = {'status': status, 'message': message}

From aabb970b75925ea5e36f8812cc50ebeeefd93445 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 09:17:45 +1200
Subject: [PATCH 13/17] Add _ReadOnlyStatusName type alias for clarity

---
 ops/model.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ops/model.py b/ops/model.py
index 0f92d7871..62c93384e 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -70,8 +70,9 @@
 _StorageDictType = Dict[str, Optional[List['Storage']]]
 _BindingDictType = Dict[Union[str, 'Relation'], 'Binding']
 
+_ReadOnlyStatusName = Literal['error', 'unknown']
 _SettableStatusName = Literal['active', 'blocked', 'maintenance', 'waiting']
-_StatusName = Union[_SettableStatusName, Literal['error', 'unknown']]
+_StatusName = Union[_SettableStatusName, _ReadOnlyStatusName]
 _StatusDict = TypedDict('_StatusDict', {'status': _StatusName, 'message': str})
 _SETTABLE_STATUS_NAMES: Tuple[_SettableStatusName, ...] = get_args(_SettableStatusName)
 

From 435e539079257b5f43499959c15d6c25cfffff1d Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 09:21:30 +1200
Subject: [PATCH 14/17] Make StatusName public (_StatusName -> StatusName)

---
 ops/model.py       | 10 +++++-----
 ops/testing.py     |  4 ++--
 test/test_charm.py | 10 +++++-----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index 62c93384e..860c822d9 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -72,8 +72,8 @@
 
 _ReadOnlyStatusName = Literal['error', 'unknown']
 _SettableStatusName = Literal['active', 'blocked', 'maintenance', 'waiting']
-_StatusName = Union[_SettableStatusName, _ReadOnlyStatusName]
-_StatusDict = TypedDict('_StatusDict', {'status': _StatusName, 'message': str})
+StatusName = Union[_SettableStatusName, _ReadOnlyStatusName]
+_StatusDict = TypedDict('_StatusDict', {'status': StatusName, 'message': str})
 _SETTABLE_STATUS_NAMES: Tuple[_SettableStatusName, ...] = get_args(_SettableStatusName)
 
 # mapping from relation name to a list of relation objects
@@ -1878,10 +1878,10 @@ class StatusBase:
     directly use the child class such as :class:`ActiveStatus` to indicate their status.
     """
 
-    _statuses: Dict[_StatusName, Type['StatusBase']] = {}
+    _statuses: Dict[StatusName, Type['StatusBase']] = {}
 
     # Subclasses must provide this attribute
-    name: _StatusName
+    name: StatusName
 
     def __init__(self, message: str = ''):
         if self.__class__ is StatusBase:
@@ -1915,7 +1915,7 @@ def from_name(cls, name: str, message: str):
             # unknown is special
             return UnknownStatus()
         else:
-            return cls._statuses[typing.cast(_StatusName, name)](message)
+            return cls._statuses[typing.cast(StatusName, name)](message)
 
     @classmethod
     def register(cls, child: Type['StatusBase']):
diff --git a/ops/testing.py b/ops/testing.py
index a35a83186..1560de548 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -60,7 +60,7 @@
 from ops._private import yaml
 from ops.charm import CharmBase, CharmMeta, RelationRole
 from ops.jujucontext import _JujuContext
-from ops.model import Container, RelationNotFoundError, _NetworkDict, _StatusName
+from ops.model import Container, RelationNotFoundError, _NetworkDict, StatusName
 from ops.pebble import ExecProcess
 
 ReadableBuffer = Union[bytes, str, StringIO, BytesIO, BinaryIO]
@@ -82,7 +82,7 @@
 _RawStatus = TypedDict(
     '_RawStatus',
     {
-        'status': _StatusName,
+        'status': StatusName,
         'message': str,
     },
 )
diff --git a/test/test_charm.py b/test/test_charm.py
index 2f399a25b..0939fbb6b 100644
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -22,7 +22,7 @@
 
 import ops
 import ops.charm
-from ops.model import ModelError, _StatusName
+from ops.model import ModelError, StatusName
 
 from .test_helpers import FakeScript, create_framework
 
@@ -965,11 +965,11 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
 def test_collect_status_priority_valid(
     request: pytest.FixtureRequest,
     fake_script: FakeScript,
-    statuses: typing.List[_StatusName],
+    statuses: typing.List[StatusName],
     expected: str,
 ):
     class MyCharm(ops.CharmBase):
-        def __init__(self, framework: ops.Framework, statuses: typing.List[_StatusName]):
+        def __init__(self, framework: ops.Framework, statuses: typing.List[StatusName]):
             super().__init__(framework)
             self.framework.observe(self.on.collect_app_status, self._on_collect_status)
             self.statuses = statuses
@@ -999,10 +999,10 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
 def test_collect_status_priority_invalid(
     request: pytest.FixtureRequest,
     fake_script: FakeScript,
-    statuses: typing.List[_StatusName],
+    statuses: typing.List[StatusName],
 ):
     class MyCharm(ops.CharmBase):
-        def __init__(self, framework: ops.Framework, statuses: typing.List[_StatusName]):
+        def __init__(self, framework: ops.Framework, statuses: typing.List[StatusName]):
             super().__init__(framework)
             self.framework.observe(self.on.collect_app_status, self._on_collect_status)
             self.statuses = statuses

From f12685341e22895ae3beee47710172ba4e1c5c22 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 09:27:50 +1200
Subject: [PATCH 15/17] Reorder StatusName in import for ruff

---
 ops/testing.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ops/testing.py b/ops/testing.py
index 1560de548..1953f27d0 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -60,7 +60,7 @@
 from ops._private import yaml
 from ops.charm import CharmBase, CharmMeta, RelationRole
 from ops.jujucontext import _JujuContext
-from ops.model import Container, RelationNotFoundError, _NetworkDict, StatusName
+from ops.model import Container, RelationNotFoundError, StatusName, _NetworkDict
 from ops.pebble import ExecProcess
 
 ReadableBuffer = Union[bytes, str, StringIO, BytesIO, BinaryIO]

From fb95f48437e73e143d38929cbe67d7b8cb10b055 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 13:48:12 +1200
Subject: [PATCH 16/17] Revert changes to docstring for CollectStatusEvent

The plan is to make using ErrorStatus and UnknownStatus here an error
---
 ops/charm.py | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/ops/charm.py b/ops/charm.py
index bc0854b2c..c626d11da 100644
--- a/ops/charm.py
+++ b/ops/charm.py
@@ -1079,17 +1079,12 @@ class CollectStatusEvent(LifecycleEvent):
 
     The order of priorities is as follows, from highest to lowest:
 
-    * error (Note: will always result in an InvalidStatusError!)
+    * error
     * blocked
     * maintenance
     * waiting
     * active
-    * unknown (Note: will result in InvalidStatusError if set as the status)
-
-    Note that "error" and "unknown" are read-only statuses, and an InvalidStatusError
-    will be raised if either of these are chosen as the highest-priority status and
-    set as the status. Note also that as "error" has the highest priority, it will
-    always cause in an InvalidStatusError.
+    * unknown
 
     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

From 2fe3bbdc763a9e7a18fdb0c4527896fa70738aee Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Wed, 18 Sep 2024 13:55:14 +1200
Subject: [PATCH 17/17] Expose model.StatusName in __init__.py

---
 ops/__init__.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ops/__init__.py b/ops/__init__.py
index 19679cfec..3108647db 100644
--- a/ops/__init__.py
+++ b/ops/__init__.py
@@ -162,6 +162,7 @@
     'SecretRotate',
     'ServiceInfoMapping',
     'StatusBase',
+    'StatusName',
     'Storage',
     'StorageMapping',
     'TooManyRelatedAppsError',
@@ -309,6 +310,7 @@
     SecretRotate,
     ServiceInfoMapping,
     StatusBase,
+    StatusName,
     Storage,
     StorageMapping,
     TooManyRelatedAppsError,