Merge lp:~jtv/maas/bug-1313556 into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Fix hanging “delete” in apiclient package.

This is a simple change, but it took me quite a while to figure out. Deleting a resource through the API client would hang while trying to read the server's response status. As it turns out, the problem wasn't so much with the response as with the request: it must have a body, even if that body should probably be empty.

Jeroen

[Gavin Panella (allenap)]

Does this boil down to a bug in a library we're using?

review: Approve

[Jeroen T. Vermeulen (jtv)]

I suspect it falls under “leaky abstraction” — technically our responsibility, but hard to be aware of until you run into trouble and figure it out.

=== modified file 'src/apiclient/maas_client.py'
--- src/apiclient/maas_client.py	2013-12-03 07:55:27 +0000
+++ src/apiclient/maas_client.py	2014-04-28 20:40:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """MAAS OAuth API connection library."""
@@ -240,5 +240,7 @@
     def delete(self, path):
         """Dispatch a DELETE on the resource at `path`."""
         url, headers, body = self._formulate_change(path, {})
+        # The body will be empty, but it must be passed.  Otherwise, the
+        # request will hang while trying to read a response (bug 1313556).
         return self.dispatcher.dispatch_query(
-            url, method="DELETE", headers=headers)
+            url, method="DELETE", headers=headers, data=body)

=== modified file 'src/apiclient/tests/test_maas_client.py'
--- src/apiclient/tests/test_maas_client.py	2013-12-03 07:55:27 +0000
+++ src/apiclient/tests/test_maas_client.py	2014-04-28 20:40:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test MAAS HTTP API client."""
@@ -346,3 +346,10 @@
         self.assertEqual(client._make_url(path), request['request_url'])
         self.assertIn('Authorization', request['headers'])
         self.assertEqual('DELETE', request['method'])
+
+    def test_delete_passes_body(self):
+        # A DELETE request should have an empty body.  But we can't just leave
+        # the body out altogether, or the request will hang (bug 1313556).
+        client = make_client()
+        client.delete(make_path())
+        self.assertIsNotNone(client.dispatcher.last_call['data'])