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

[ubuntudotcom1]

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

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

[Gavin Panella (allenap)]

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

review: Needs Information

[Raphaël Badin (rvb)]

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.

[Raphaël Badin (rvb)]

review: Needs Information

Unmerged revisions

3814. By ubuntudotcom1 on 2015-04-10

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

=== modified file 'src/maasserver/api/tests/test_api.py'
--- src/maasserver/api/tests/test_api.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/api/tests/test_api.py	2015-04-10 16:21:54 +0000
@@ -7,7 +7,7 @@
     absolute_import,
     print_function,
     unicode_literals,
-    )
+)
 
 str = None
 
@@ -45,6 +45,7 @@
 from maasserver.testing.orm import reload_object
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils.orm import get_one
+from maasserver.utils.views import HttpResponseConflict
 from maastesting.djangotestcase import DjangoTransactionTestCase
 from mock import Mock
 from netaddr import IPAddress
@@ -69,7 +70,7 @@
         expected = (
             Equals(httplib.UNAUTHORIZED),
             Contains("Invalid access token:"),
-            )
+        )
         self.assertThat(observed, MatchesListwise(expected))
 
 
@@ -121,7 +122,7 @@
         self.request.POST = {
             "power_type": power_type,
             "power_parameters": json.dumps(power_parameters),
-            }
+        }
         store_node_power_parameters(self.node, self.request)
         self.assertEqual(power_type, self.node.power_type)
         self.assertEqual(power_parameters, self.node.power_parameters)
@@ -134,7 +135,7 @@
         self.request.POST = {
             "power_type": power_type,
             "power_parameters": "Not JSON.",
-            }
+        }
         self.assertRaises(
             MAASAPIBadRequest, store_node_power_parameters,
             self.node, self.request)
@@ -156,7 +157,7 @@
         power_type = ''
         self.request.POST = {
             "power_type": '',
-            }
+        }
         store_node_power_parameters(self.node, self.request)
         self.assertEqual(power_type, self.node.power_type)
         self.save.assert_called_once_with()
@@ -224,13 +225,13 @@
                 id=keys[0].id,
                 key=keys[0].key,
                 resource_uri=reverse('sshkey_handler', args=[keys[0].id]),
-                ),
+            ),
             dict(
                 id=keys[1].id,
                 key=keys[1].key,
                 resource_uri=reverse('sshkey_handler', args=[keys[1].id]),
-                ),
-            ]
+            ),
+        ]
         self.assertEqual(expected_result, parsed_result)
 
     def test_get_by_id_works(self):
@@ -245,7 +246,7 @@
             id=key.id,
             key=key.key,
             resource_uri=reverse('sshkey_handler', args=[key.id]),
-            )
+        )
         self.assertEqual(expected, parsed_result)
 
     def test_delete_by_id_works(self):
@@ -313,7 +314,6 @@
 
 
 class MAASAPITest(APITestCase):
-
     def test_handler_path(self):
         self.assertEqual(
             '/api/1.0/maas/', reverse('maas_handler'))
@@ -470,6 +470,12 @@
             (httplib.INTERNAL_SERVER_ERROR, error_message),
             (response.status_code, response.content))
 
+    def test_translates_http_409_conflict(self):
+        self.patch(nodes_module,
+                   'create_node').side_effect = HttpResponseConflict()
+        response = self.client.post(reverse('nodes_handler'), {'op': 'new'})
+        self.assertEqual(httplib.CONFLICT, response.status_code)
+
 
 def dict_subset(obj, fields):
     """Return a dict of a subset of the fields/values of an object."""
 
=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/exceptions.py	2015-04-10 16:21:54 +0000
@@ -7,7 +7,7 @@
     absolute_import,
     print_function,
     unicode_literals,
-    )
+)
 
 str = None
 
@@ -26,7 +26,7 @@
     "StaticIPAddressExhaustion",
     "StaticIPAddressTypeClash",
     "UnresolvableHost",
-    ]
+]
 
 
 import httplib
@@ -110,6 +110,11 @@
     api_error = httplib.CONFLICT
 
 
+class MAASAPIMaxRetryAttempts(MAASAPIException):
+    """The api has reached the maximum number of retry attempts."""
+    api_error = httplib.CONFLICT
+
+
 class NodesNotAvailable(NodeStateViolation):
     """Requested node(s) are not available to be acquired."""
     api_error = httplib.CONFLICT
 
=== modified file 'src/maasserver/utils/views.py'
--- src/maasserver/utils/views.py	2015-04-09 15:53:25 +0000
+++ src/maasserver/utils/views.py	2015-04-10 16:21:54 +0000
@@ -28,6 +28,7 @@
 from django.core.urlresolvers import get_resolver
 from django.db import transaction
 from django.template.response import SimpleTemplateResponse
+from maasserver.exceptions import MAASAPIMaxRetryAttempts
 from maasserver.utils.orm import (
     gen_retry_intervals,
     is_serialization_failure,
@@ -124,7 +125,8 @@
                 self.content = response.content
 
 
-class HttpResponseConflict(MAASDjangoTemplateResponse):
+class HttpResponseConflict(
+        MAASAPIMaxRetryAttempts, MAASDjangoTemplateResponse):
     status_code = httplib.CONFLICT