launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/maasclient-expose-op into lp:maas.

Commit message:
Change MAASClient.post() so that it exposes the operation in the URL rather than hiding it in the POST body.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/maasclient-expose-op/+merge/128180

I thought this was a patch someone else did, but the client doesn't do it this way right now.

This changes MAASClient.post() so that it still encodes 'op' parameters in the URL, rather than in the body.

This is an optional change, but it makes debugging the log files a lot nicer, because rather than just seeing POST .../object/
you see POST ../object/?op=action
So it is clear what actions are being performed on the object in question.

-- 
https://code.launchpad.net/~jameinel/maas/maasclient-expose-op/+merge/128180
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/maas/maasclient-expose-op into lp:maas.

=== modified file 'src/apiclient/maas_client.py'
--- src/apiclient/maas_client.py	2012-09-25 03:51:13 +0000
+++ src/apiclient/maas_client.py	2012-10-05 07:42:23 +0000
@@ -149,6 +149,10 @@
         :return: A tuple: URL, headers, and body for the request.
         """
         url = self._make_url(path)
+        if 'op' in params:
+            params = dict(params)
+            op = params.pop('op')
+            url += '?' + urlencode({'op': op})
         body, headers = encode_multipart_data(params, {})
         self.auth.sign_request(url, headers)
         return url, headers, body

=== modified file 'src/apiclient/tests/test_maas_client.py'
--- src/apiclient/tests/test_maas_client.py	2012-09-22 19:42:41 +0000
+++ src/apiclient/tests/test_maas_client.py	2012-10-05 07:42:23 +0000
@@ -186,9 +186,11 @@
     def test_post_dispatches_to_resource(self):
         path = make_path()
         client = make_client()
-        client.post(path, factory.getRandomString())
+        method = factory.getRandomString()
+        client.post(path, method)
         request = client.dispatcher.last_call
-        self.assertEqual(client._make_url(path), request['request_url'])
+        self.assertEqual(client._make_url(path) + "?op=%s" % (method,),
+                         request['request_url'])
         self.assertIn('Authorization', request['headers'])
         self.assertEqual('POST', request['method'])
 
@@ -200,7 +202,8 @@
         request = client.dispatcher.last_call
         post, _ = parse_headers_and_body_with_django(
             request["headers"], request["data"])
-        self.assertEqual({"parameter": [param], "op": [method]}, post)
+        self.assertTrue(request["request_url"].endswith('?op=%s' % (method,)))
+        self.assertEqual({"parameter": [param]}, post)
 
     def test_put_dispatches_to_resource(self):
         path = make_path()