Merge lp:~ubuntudotcom1/maas/event-api-test-409 into lp:~maas-committers/maas/trunk

Proposed by ubuntudotcom1
Status: Rejected
Rejected by: Andres Rodriguez
Proposed branch: lp:~ubuntudotcom1/maas/event-api-test-409
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 157 lines (+26/-13)
3 files modified
src/maasserver/api/tests/test_api.py (+16/-10)
src/maasserver/exceptions.py (+7/-2)
src/maasserver/utils/views.py (+3/-1)
To merge this branch: bzr merge lp:~ubuntudotcom1/maas/event-api-test-409
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Needs Information
Gavin Panella (community) Needs Information
Review via email: mp+255862@code.launchpad.net

Commit message

Make HttpResponseConflict also inherit from MAASAPIMaxRetryAttempts so that it may be handled by the api.

Description of the change

Make HttpResponseConflict also inherit from MAASAPIMaxRetryAttempts so that it may be handled by the api.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

I don't fully understand why this is needed. I'd like to get a second opinion from Raphaël.

review: Needs Information
Revision history for this message
Raphaël Badin (rvb) wrote :

Agree with Gavin's remark: I'm not sure having a specific template for 409 exceptions is a good idea: it mixes things up (but maybe the code could be fixed) but also it's mostly useless: most of the code that's susceptible to raise said exception has been "websocketified" already.

Additionally (and as discussed in Nuremberg), we should see if it isn't possible to put all this exception-handling logic in src/maasserver/middleware.py; this is where all the other exception-handling happens. This is mostly a problem with the previous branch (the one whose behavior this branches fixes), not this one.

Revision history for this message
Raphaël Badin (rvb) :
review: Needs Information

Unmerged revisions

3814. By ubuntudotcom1

Make HttpResponseConflict also inherit from MAASAPIMaxRetryAttempts so that it may be handled by the api.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/tests/test_api.py'
2--- src/maasserver/api/tests/test_api.py 2015-03-25 15:33:23 +0000
3+++ src/maasserver/api/tests/test_api.py 2015-04-10 16:21:54 +0000
4@@ -7,7 +7,7 @@
5 absolute_import,
6 print_function,
7 unicode_literals,
8- )
9+)
10
11 str = None
12
13@@ -45,6 +45,7 @@
14 from maasserver.testing.orm import reload_object
15 from maasserver.testing.testcase import MAASServerTestCase
16 from maasserver.utils.orm import get_one
17+from maasserver.utils.views import HttpResponseConflict
18 from maastesting.djangotestcase import DjangoTransactionTestCase
19 from mock import Mock
20 from netaddr import IPAddress
21@@ -69,7 +70,7 @@
22 expected = (
23 Equals(httplib.UNAUTHORIZED),
24 Contains("Invalid access token:"),
25- )
26+ )
27 self.assertThat(observed, MatchesListwise(expected))
28
29
30@@ -121,7 +122,7 @@
31 self.request.POST = {
32 "power_type": power_type,
33 "power_parameters": json.dumps(power_parameters),
34- }
35+ }
36 store_node_power_parameters(self.node, self.request)
37 self.assertEqual(power_type, self.node.power_type)
38 self.assertEqual(power_parameters, self.node.power_parameters)
39@@ -134,7 +135,7 @@
40 self.request.POST = {
41 "power_type": power_type,
42 "power_parameters": "Not JSON.",
43- }
44+ }
45 self.assertRaises(
46 MAASAPIBadRequest, store_node_power_parameters,
47 self.node, self.request)
48@@ -156,7 +157,7 @@
49 power_type = ''
50 self.request.POST = {
51 "power_type": '',
52- }
53+ }
54 store_node_power_parameters(self.node, self.request)
55 self.assertEqual(power_type, self.node.power_type)
56 self.save.assert_called_once_with()
57@@ -224,13 +225,13 @@
58 id=keys[0].id,
59 key=keys[0].key,
60 resource_uri=reverse('sshkey_handler', args=[keys[0].id]),
61- ),
62+ ),
63 dict(
64 id=keys[1].id,
65 key=keys[1].key,
66 resource_uri=reverse('sshkey_handler', args=[keys[1].id]),
67- ),
68- ]
69+ ),
70+ ]
71 self.assertEqual(expected_result, parsed_result)
72
73 def test_get_by_id_works(self):
74@@ -245,7 +246,7 @@
75 id=key.id,
76 key=key.key,
77 resource_uri=reverse('sshkey_handler', args=[key.id]),
78- )
79+ )
80 self.assertEqual(expected, parsed_result)
81
82 def test_delete_by_id_works(self):
83@@ -313,7 +314,6 @@
84
85
86 class MAASAPITest(APITestCase):
87-
88 def test_handler_path(self):
89 self.assertEqual(
90 '/api/1.0/maas/', reverse('maas_handler'))
91@@ -470,6 +470,12 @@
92 (httplib.INTERNAL_SERVER_ERROR, error_message),
93 (response.status_code, response.content))
94
95+ def test_translates_http_409_conflict(self):
96+ self.patch(nodes_module,
97+ 'create_node').side_effect = HttpResponseConflict()
98+ response = self.client.post(reverse('nodes_handler'), {'op': 'new'})
99+ self.assertEqual(httplib.CONFLICT, response.status_code)
100+
101
102 def dict_subset(obj, fields):
103 """Return a dict of a subset of the fields/values of an object."""
104
105=== modified file 'src/maasserver/exceptions.py'
106--- src/maasserver/exceptions.py 2015-03-25 15:33:23 +0000
107+++ src/maasserver/exceptions.py 2015-04-10 16:21:54 +0000
108@@ -7,7 +7,7 @@
109 absolute_import,
110 print_function,
111 unicode_literals,
112- )
113+)
114
115 str = None
116
117@@ -26,7 +26,7 @@
118 "StaticIPAddressExhaustion",
119 "StaticIPAddressTypeClash",
120 "UnresolvableHost",
121- ]
122+]
123
124
125 import httplib
126@@ -110,6 +110,11 @@
127 api_error = httplib.CONFLICT
128
129
130+class MAASAPIMaxRetryAttempts(MAASAPIException):
131+ """The api has reached the maximum number of retry attempts."""
132+ api_error = httplib.CONFLICT
133+
134+
135 class NodesNotAvailable(NodeStateViolation):
136 """Requested node(s) are not available to be acquired."""
137 api_error = httplib.CONFLICT
138
139=== modified file 'src/maasserver/utils/views.py'
140--- src/maasserver/utils/views.py 2015-04-09 15:53:25 +0000
141+++ src/maasserver/utils/views.py 2015-04-10 16:21:54 +0000
142@@ -28,6 +28,7 @@
143 from django.core.urlresolvers import get_resolver
144 from django.db import transaction
145 from django.template.response import SimpleTemplateResponse
146+from maasserver.exceptions import MAASAPIMaxRetryAttempts
147 from maasserver.utils.orm import (
148 gen_retry_intervals,
149 is_serialization_failure,
150@@ -124,7 +125,8 @@
151 self.content = response.content
152
153
154-class HttpResponseConflict(MAASDjangoTemplateResponse):
155+class HttpResponseConflict(
156+ MAASAPIMaxRetryAttempts, MAASDjangoTemplateResponse):
157 status_code = httplib.CONFLICT
158
159