From 18f09e8fa07b993e485fadb9d38c7b0c3b9f67f8 Mon Sep 17 00:00:00 2001 From: James Garner Date: Mon, 23 Sep 2024 09:48:51 +1200 Subject: [PATCH 1/4] Deprecate and document StatusBase.register --- ops/model.py | 30 +++++++++++++++++++++++++++--- test/test_model.py | 3 +++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/ops/model.py b/ops/model.py index 6405e7080..3ea5338c4 100644 --- a/ops/model.py +++ b/ops/model.py @@ -1908,7 +1908,7 @@ def __init__(self, message: str = ''): self.message = message def __init_subclass__(cls): - StatusBase.register(cls) + StatusBase._register(cls) def __eq__(self, other: 'StatusBase') -> bool: if not isinstance(self, type(other)): @@ -1941,14 +1941,38 @@ def from_name(cls, name: str, message: str): @classmethod def register(cls, child: Type['StatusBase']): - """Register a Status for the child's name.""" + """Class decorator to register a subclass for lookup using :meth:`from_name`. + + Note: this method is deprecated. Registration is now automatic via __init_subclass. + Also, this decorator obscures the class type when used as a decorator. + + Note: this method is intended for internal use only. + It is used to make the valid Juju statuses available by name. + + Args: + child: A subclass of StatusBase, with a ``name`` attribute of type ``str``. + + Returns: + The decorated class, unmodified. + """ + warnings.warn( + 'StatusBase.register should not be called. It is intended' + 'for internal use only and is superseded by automatic subclass' + 'registration via __init_subclass__', + DeprecationWarning, + stacklevel=2, + ) + cls._register(child) + return child + + @classmethod + def _register(cls, child: Type['StatusBase']) -> None: 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', ) cls._statuses[child.name] = child - return child _priorities = { 'error': 5, diff --git a/test/test_model.py b/test/test_model.py index 1f0b6391c..72f380222 100644 --- a/test/test_model.py +++ b/test/test_model.py @@ -830,6 +830,9 @@ class NoNameStatus(ops.StatusBase): # pyright: ignore[reportUnusedClass] class NonStringNameStatus(ops.StatusBase): # pyright: ignore[reportUnusedClass] name = None # pyright: ignore[reportAssignmentType] + with pytest.deprecated_call(): + ops.StatusBase.register(ops.ActiveStatus) + def test_status_repr(self): test_cases = { "ActiveStatus('Seashell')": ops.ActiveStatus('Seashell'), From 75eb180c417d35e8cb863443bb0f1eec60d0d8f0 Mon Sep 17 00:00:00 2001 From: James Garner Date: Mon, 23 Sep 2024 10:52:02 +1200 Subject: [PATCH 2/4] Cleaner user facing documentation and deprecation warning --- ops/model.py | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/ops/model.py b/ops/model.py index 3ea5338c4..da00d756a 100644 --- a/ops/model.py +++ b/ops/model.py @@ -1941,27 +1941,8 @@ def from_name(cls, name: str, message: str): @classmethod def register(cls, child: Type['StatusBase']): - """Class decorator to register a subclass for lookup using :meth:`from_name`. - - Note: this method is deprecated. Registration is now automatic via __init_subclass. - Also, this decorator obscures the class type when used as a decorator. - - Note: this method is intended for internal use only. - It is used to make the valid Juju statuses available by name. - - Args: - child: A subclass of StatusBase, with a ``name`` attribute of type ``str``. - - Returns: - The decorated class, unmodified. - """ - warnings.warn( - 'StatusBase.register should not be called. It is intended' - 'for internal use only and is superseded by automatic subclass' - 'registration via __init_subclass__', - DeprecationWarning, - stacklevel=2, - ) + """.. deprecated:: 2.17.0 Deprecated - this was for internal use only.""" + warnings.warn('StatusBase.register is for internal use only', DeprecationWarning) cls._register(child) return child From 6d11904030adc67091f1274441384a71ab9e3183 Mon Sep 17 00:00:00 2001 From: James Garner Date: Mon, 23 Sep 2024 11:14:40 +1200 Subject: [PATCH 3/4] Use stacklevel 2 --- ops/model.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ops/model.py b/ops/model.py index da00d756a..d03c4b584 100644 --- a/ops/model.py +++ b/ops/model.py @@ -1942,7 +1942,9 @@ def from_name(cls, name: str, message: str): @classmethod def register(cls, child: Type['StatusBase']): """.. deprecated:: 2.17.0 Deprecated - this was for internal use only.""" - warnings.warn('StatusBase.register is for internal use only', DeprecationWarning) + warnings.warn( + 'StatusBase.register is for internal use only', DeprecationWarning, stacklevel=2 + ) cls._register(child) return child From 3d086a74367922a631450878066fb3e64a5b8d0b Mon Sep 17 00:00:00 2001 From: James Garner Date: Mon, 23 Sep 2024 11:15:59 +1200 Subject: [PATCH 4/4] Make StatusBase.register deprecated check a separate test --- test/test_model.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_model.py b/test/test_model.py index 72f380222..015dbc0ee 100644 --- a/test/test_model.py +++ b/test/test_model.py @@ -830,6 +830,7 @@ class NoNameStatus(ops.StatusBase): # pyright: ignore[reportUnusedClass] class NonStringNameStatus(ops.StatusBase): # pyright: ignore[reportUnusedClass] name = None # pyright: ignore[reportAssignmentType] + def test_base_status_register_is_deprecated(self): with pytest.deprecated_call(): ops.StatusBase.register(ops.ActiveStatus)