Merge ~lloydwaltersj/maas:deprecation-marker into maas:master
- Git
- lp:~lloydwaltersj/maas
- deprecation-marker
- Merge into master
Status: | Merged |
---|---|
Approved by: | Jack Lloyd-Walters |
Approved revision: | 157ceeecc564aa0d9b116a014c861faa4f0e77a3 |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~lloydwaltersj/maas:deprecation-marker |
Merge into: | maas:master |
Diff against target: |
303 lines (+109/-22) 7 files modified
src/maasserver/api/commissioning_scripts.py (+6/-7) src/maasserver/api/networks.py (+10/-6) src/maasserver/api/nodes.py (+2/-0) src/maasserver/api/pods.py (+8/-8) src/maasserver/api/support.py (+56/-1) src/maasserver/api/tests/test_support.py (+23/-0) src/maasserver/exceptions.py (+4/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Adam Collard (community) | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+429164@code.launchpad.net |
Commit message
Add deprecated decorator for use with MAAS Api
Description of the change
Api endpoints and methods that are deprecated have very freeform markings in docstrings and endpoint names.
This should standardise that, as well as expose a new "deprecated" attribute that can be used to programatically determine if an operation/endpoint is deprecated.
- ed7efec... by Jack Lloyd-Walters
-
Change how docstring is modified
- 6a8e4e9... by Jack Lloyd-Walters
-
formatting
- c9655ff... by Jack Lloyd-Walters
-
remove leftover print
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: c9655ff1871ecb5
- 6454d9a... by Jack Lloyd-Walters
-
use func_name for function name
- e1e319c... by Jack Lloyd-Walters
-
Change docstring ordering
- 80bed44... by Jack Lloyd-Walters
-
formatting
- 7c8b7bf... by Jack Lloyd-Walters
-
change if-elif-if to if-elif-elise
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 6454d9ad27a9beb
Alberto Donato (ack) : | # |
Jack Lloyd-Walters (lloydwaltersj) : | # |
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 7c8b7bf7767392a
- 9327ce1... by Jack Lloyd-Walters
-
apply suggestions
Adam Collard (adam-collard) : | # |
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 9327ce1ca96652a
- 0f2e286... by Jack Lloyd-Walters
-
seperate methods to handle docstrings
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 0f2e286bcca3224
Jack Lloyd-Walters (lloydwaltersj) wrote : | # |
jenkins: !test
- 4938600... by Jack Lloyd-Walters
-
bring branch up to parity
- 157ceee... by Jack Lloyd-Walters
-
Change order
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 0f2e286bcca3224
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 157ceeecc564aa0
Adam Collard (adam-collard) : | # |
Preview Diff
1 | diff --git a/src/maasserver/api/commissioning_scripts.py b/src/maasserver/api/commissioning_scripts.py |
2 | index 11b2981..477b688 100644 |
3 | --- a/src/maasserver/api/commissioning_scripts.py |
4 | +++ b/src/maasserver/api/commissioning_scripts.py |
5 | @@ -11,7 +11,8 @@ from django.shortcuts import get_object_or_404 |
6 | from django.urls import reverse |
7 | from piston3.utils import rc |
8 | |
9 | -from maasserver.api.support import OperationsHandler |
10 | +from maasserver.api.scripts import NodeScriptHandler, NodeScriptsHandler |
11 | +from maasserver.api.support import deprecated, OperationsHandler |
12 | from maasserver.api.utils import get_mandatory_param |
13 | from maasserver.audit import create_audit_event |
14 | from maasserver.enum import ENDPOINT |
15 | @@ -29,16 +30,15 @@ def get_content_parameter(request): |
16 | return content_file.read() |
17 | |
18 | |
19 | +@deprecated(use=NodeScriptsHandler) |
20 | class CommissioningScriptsHandler(OperationsHandler): |
21 | """ |
22 | Manage custom commissioning scripts. |
23 | |
24 | This functionality is only available to administrators. |
25 | - |
26 | - This endpoint has been deprecated in favor of the node-scripts endpoint. |
27 | """ |
28 | |
29 | - api_doc_section_name = "Commissioning scripts (deprecated)" |
30 | + api_doc_section_name = "Commissioning scripts" |
31 | |
32 | update = delete = None |
33 | |
34 | @@ -109,16 +109,15 @@ class CommissioningScriptsHandler(OperationsHandler): |
35 | return ("commissioning_scripts_handler", []) |
36 | |
37 | |
38 | +@deprecated(use=NodeScriptHandler) |
39 | class CommissioningScriptHandler(OperationsHandler): |
40 | """ |
41 | Manage a custom commissioning script. |
42 | |
43 | This functionality is only available to administrators. |
44 | - |
45 | - This endpoint has been deprecated in favor of the node-script endpoint. |
46 | """ |
47 | |
48 | - api_doc_section_name = "Commissioning script (deprecated)" |
49 | + api_doc_section_name = "Commissioning script" |
50 | |
51 | # Relies on Piston's built-in DELETE implementation. There is no POST. |
52 | create = None |
53 | diff --git a/src/maasserver/api/networks.py b/src/maasserver/api/networks.py |
54 | index 0810bd0..bbe0eb8 100644 |
55 | --- a/src/maasserver/api/networks.py |
56 | +++ b/src/maasserver/api/networks.py |
57 | @@ -7,7 +7,13 @@ |
58 | from django.urls import reverse |
59 | from piston3.utils import rc |
60 | |
61 | -from maasserver.api.support import admin_method, operation, OperationsHandler |
62 | +from maasserver.api.subnets import SubnetHandler, SubnetsHandler |
63 | +from maasserver.api.support import ( |
64 | + admin_method, |
65 | + deprecated, |
66 | + operation, |
67 | + OperationsHandler, |
68 | +) |
69 | from maasserver.exceptions import MAASAPIValidationError |
70 | from maasserver.forms import NetworksListingForm |
71 | from maasserver.models import Interface, Node, Subnet |
72 | @@ -39,14 +45,13 @@ def render_networks_json(subnets): |
73 | return [render_network_json(subnet) for subnet in subnets] |
74 | |
75 | |
76 | +@deprecated(use=SubnetHandler) |
77 | class NetworkHandler(OperationsHandler): |
78 | """ |
79 | Manage a network. |
80 | - |
81 | - This endpoint is deprecated. Use the new 'subnet' endpoint instead. |
82 | """ |
83 | |
84 | - api_doc_section_name = "Network (deprecated)" |
85 | + api_doc_section_name = "Network" |
86 | create = None |
87 | |
88 | def read(self, request, name): |
89 | @@ -150,11 +155,10 @@ class NetworkHandler(OperationsHandler): |
90 | return ("network_handler", (name,)) |
91 | |
92 | |
93 | +@deprecated(use=SubnetsHandler) |
94 | class NetworksHandler(OperationsHandler): |
95 | """ |
96 | Manage the networks. |
97 | - |
98 | - This endpoint is deprecated. Use the new 'subnets' endpoint instead. |
99 | """ |
100 | |
101 | api_doc_section_name = "Networks" |
102 | diff --git a/src/maasserver/api/nodes.py b/src/maasserver/api/nodes.py |
103 | index 3ff069c..04c2457 100644 |
104 | --- a/src/maasserver/api/nodes.py |
105 | +++ b/src/maasserver/api/nodes.py |
106 | @@ -20,6 +20,7 @@ from piston3.utils import rc |
107 | from maasserver.api.support import ( |
108 | admin_method, |
109 | AnonymousOperationsHandler, |
110 | + deprecated, |
111 | operation, |
112 | OperationsHandler, |
113 | ) |
114 | @@ -914,6 +915,7 @@ class WorkloadAnnotationsMixin: |
115 | return node |
116 | |
117 | @operation(idempotent=False) |
118 | + @deprecated(use=set_workload_annotations) |
119 | def set_owner_data(self, request, system_id): |
120 | """@description-title Deprecated, use set-workload-annotations. |
121 | @description Deprecated, use set-workload-annotations instead.""" |
122 | diff --git a/src/maasserver/api/pods.py b/src/maasserver/api/pods.py |
123 | index 8af6775..58c8910 100644 |
124 | --- a/src/maasserver/api/pods.py |
125 | +++ b/src/maasserver/api/pods.py |
126 | @@ -9,7 +9,12 @@ from django.urls import reverse |
127 | from formencode.validators import String |
128 | from piston3.utils import rc |
129 | |
130 | -from maasserver.api.support import admin_method, operation, OperationsHandler |
131 | +from maasserver.api.support import ( |
132 | + admin_method, |
133 | + deprecated, |
134 | + operation, |
135 | + OperationsHandler, |
136 | +) |
137 | from maasserver.api.utils import get_mandatory_param |
138 | from maasserver.exceptions import MAASAPIValidationError, PodProblem |
139 | from maasserver.forms.pods import ComposeMachineForm, DeletePodForm, PodForm |
140 | @@ -423,12 +428,10 @@ class VmHostHandler(OperationsHandler): |
141 | |
142 | # Pods are being renamed to VM hosts. Keep the old name on the API as well for |
143 | # backwards-compatibility. |
144 | +@deprecated(use=VmHostHandler) |
145 | class PodHandler(VmHostHandler): |
146 | """ |
147 | Manage an individual Pod. |
148 | - |
149 | - The /pod/ API endpoint is deprecated. Please use the /vm-host/ one instead. |
150 | - |
151 | """ |
152 | |
153 | api_doc_section_name = "Pod" |
154 | @@ -531,13 +534,10 @@ class VmHostsHandler(OperationsHandler): |
155 | |
156 | # Pods are being renamed to VM hosts. Keep the old name on the API as well for |
157 | # backwards-compatibility. |
158 | +@deprecated(use=VmHostsHandler) |
159 | class PodsHandler(VmHostsHandler): |
160 | """ |
161 | Manage the collection of all the pods in the MAAS. |
162 | - |
163 | - The /pods/ API endpoint is deprecated. Please use the /vm-hosts/ one |
164 | - instead. |
165 | - |
166 | """ |
167 | |
168 | api_doc_section_name = "Pods" |
169 | diff --git a/src/maasserver/api/support.py b/src/maasserver/api/support.py |
170 | index d86502f..db8b222 100644 |
171 | --- a/src/maasserver/api/support.py |
172 | +++ b/src/maasserver/api/support.py |
173 | @@ -14,6 +14,7 @@ __all__ = [ |
174 | |
175 | from abc import ABCMeta, abstractproperty |
176 | from functools import wraps |
177 | +import inspect |
178 | import re |
179 | |
180 | from django.core.exceptions import PermissionDenied |
181 | @@ -26,7 +27,11 @@ from piston3.resource import Resource |
182 | from piston3.utils import HttpStatusCode, rc |
183 | |
184 | from maasserver.api.doc import get_api_description |
185 | -from maasserver.exceptions import MAASAPIBadRequest, MAASAPIValidationError |
186 | +from maasserver.exceptions import ( |
187 | + MAASAPIBadRequest, |
188 | + MAASAPIValidationError, |
189 | + MAASBadDeprecation, |
190 | +) |
191 | from maasserver.utils.orm import get_one |
192 | from provisioningserver.logger import LegacyLogger |
193 | |
194 | @@ -142,6 +147,56 @@ def operation(idempotent, exported_as=None): |
195 | return _decorator |
196 | |
197 | |
198 | +def deprecated(use): |
199 | + """Decorator to note a method or class is deprecated on the API. |
200 | + |
201 | + :param use: Name of the method or class that should be used in place of this method""" |
202 | + |
203 | + # TODO: Determine any other behaviours we might want from this decorator in future |
204 | + |
205 | + def update_method_docstring(func): |
206 | + doc = func.__doc__ if func.__doc__ else "" |
207 | + depr = f"This operation has been deprecated in favour of '{func_name(use)} {func_name(func.deprecated)}'" |
208 | + if "@description" in doc: |
209 | + description = doc.split("@description ")[1].split(".")[0] |
210 | + func.__doc = doc.replace(description, f"{description}. {depr}") |
211 | + else: |
212 | + func.__doc__ = f"{doc}\n{depr}." |
213 | + |
214 | + def update_class_docstring(cls): |
215 | + cls.__doc__ = f"{cls.__doc__}\nThe '{func_name(cls)}' endpoint has been deprecated in favour of '{func_name(use)}'." |
216 | + if "(deprecated)" not in cls.api_doc_section_name: |
217 | + cls.api_doc_section_name += " (deprecated)" |
218 | + |
219 | + def func_name(func): |
220 | + return re.sub( |
221 | + "[_ ]", "-", getattr(func, "api_doc_section_name", func.__name__) |
222 | + ) |
223 | + |
224 | + def _decorator(func): |
225 | + if type(func) is not type(use): |
226 | + raise MAASBadDeprecation( |
227 | + f"{func_name(func)} is deprecated in favour of {func_name(use)}, but {type(use)} is not a valid replacement for {type(func)}" |
228 | + ) |
229 | + func.deprecated = use |
230 | + # if an endpoint (class) was passed to the decorator, apply the decorator to all of it's methods too |
231 | + if isinstance(func, type): |
232 | + apply_to_methods(func) |
233 | + update_class_docstring(func) |
234 | + else: |
235 | + update_method_docstring(func) |
236 | + return func |
237 | + |
238 | + def apply_to_methods(cls): |
239 | + for name, method in inspect.getmembers(cls, inspect.isfunction): |
240 | + new_method = getattr(use, name, None) |
241 | + if new_method: |
242 | + method.deprecated = new_method |
243 | + update_method_docstring(method) |
244 | + |
245 | + return _decorator |
246 | + |
247 | + |
248 | METHOD_RESERVED_ADMIN = "This method is reserved for admin users." |
249 | |
250 | |
251 | diff --git a/src/maasserver/api/tests/test_support.py b/src/maasserver/api/tests/test_support.py |
252 | index 62fd55e..bbc04c9 100644 |
253 | --- a/src/maasserver/api/tests/test_support.py |
254 | +++ b/src/maasserver/api/tests/test_support.py |
255 | @@ -15,6 +15,7 @@ from maasserver.api.doc import get_api_description |
256 | from maasserver.api.support import ( |
257 | admin_method, |
258 | AdminRestrictedResource, |
259 | + deprecated, |
260 | Emitter, |
261 | OperationsHandlerMixin, |
262 | OperationsResource, |
263 | @@ -180,6 +181,28 @@ class TestAdminMethodDecorator(MAASServerTestCase): |
264 | self.assertEqual((return_value, [call()]), (response, mock.mock_calls)) |
265 | |
266 | |
267 | +class TestDeprecatedMethodDecorator(MAASServerTestCase): |
268 | + def test_function_marked_as_deprecated(self): |
269 | + def api_replacement_method(self, request): |
270 | + pass |
271 | + |
272 | + @deprecated(use=api_replacement_method) |
273 | + def api_method(self, request): |
274 | + pass |
275 | + |
276 | + self.assertEqual(api_method.deprecated, api_replacement_method) |
277 | + |
278 | + def test_class_marked_as_deprecated(self): |
279 | + class api_replacement_endpoint: |
280 | + api_doc_section_name = "Replacement" |
281 | + |
282 | + @deprecated(use=api_replacement_endpoint) |
283 | + class api_endpoint: |
284 | + api_doc_section_name = "Deprecated" |
285 | + |
286 | + self.assertEqual(api_endpoint.deprecated, api_replacement_endpoint) |
287 | + |
288 | + |
289 | class TestOperationsHandlerMixin(MAASTestCase): |
290 | """Tests for :py:class:`maasserver.api.support.OperationsHandlerMixin`.""" |
291 | |
292 | diff --git a/src/maasserver/exceptions.py b/src/maasserver/exceptions.py |
293 | index b811206..4ebae20 100644 |
294 | --- a/src/maasserver/exceptions.py |
295 | +++ b/src/maasserver/exceptions.py |
296 | @@ -245,3 +245,7 @@ class StorageClearProblem(MAASAPIException): |
297 | |
298 | class NetworkingResetProblem(MAASException): |
299 | """Raised when an issue occurs that prevents resetting networking configuration.""" |
300 | + |
301 | + |
302 | +class MAASBadDeprecation(MAASException): |
303 | + """Raised when an API endpoint or operation has it's deprecation incorrectly assigned.""" |
UNIT TESTS
-b deprecation-marker lp:~lloydwaltersj/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED maas-ci. internal: 8080/job/ maas-tester/ 437/consoleText 52cdb7899d3629f 205ded487a
LOG: http://
COMMIT: 048b4f3cd370b5e