launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/tags-use-json into lp:maas with lp:~jameinel/maas/allow-json as a prerequisite.

Commit message:
Change 2 Tag related APIs so that the server supports JSON as the POST, and the client POSTs using JSON.

This makes processing 12,000 nodes go from 45s down to 35s. Which is a pretty substantial win.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/tags-use-json/+merge/128516

See commit message.

I also tested it to make sure that with as_json=True or False the API still works. It is just faster if you use True.
-- 
https://code.launchpad.net/~jameinel/maas/tags-use-json/+merge/128516
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/maas/tags-use-json into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-10-08 11:46:58 +0000
+++ src/maasserver/api.py	2012-10-08 15:06:24 +0000
@@ -1197,7 +1197,12 @@
         to be stored in the cluster controllers (nodegroup) instead of the
         master controller.
         """
-        system_ids = request.POST.getlist('system_ids')
+        # If sent as x-www-form-urlencoded data, it comes in as a MultiDict, if
+        # sent as json it comes in as a simple dict(list).
+        if getattr(request.data, 'getlist', None) is not None:
+            system_ids = request.data.getlist('system_ids')
+        else:
+            system_ids = request.data['system_ids']
         nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
         check_nodegroup_access(request, nodegroup)
         nodes = Node.objects.filter(
@@ -1409,7 +1414,12 @@
         return Tag.objects.get_nodes(name, user=request.user)
 
     def _get_nodes_for(self, request, param, nodegroup):
-        system_ids = get_optional_list(request.POST, param)
+        # The application/x-www-form-urlencoded handler uses a MultiDict, but
+        # the application/json handler uses just a dict(key=[list])
+        if getattr(request.data, 'getlist', None) is not None:
+            system_ids = request.data.getlist(param)
+        else:
+            system_ids = request.data[param]
         if system_ids:
             nodes = Node.objects.filter(system_id__in=system_ids)
             if nodegroup is not None:
@@ -1441,7 +1451,7 @@
         tag = Tag.objects.get_tag_or_404(name=name, user=request.user)
         nodegroup = None
         if not request.user.is_superuser:
-            uuid = request.POST.get('nodegroup', None)
+            uuid = request.data.get('nodegroup', None)
             if uuid is None:
                 raise PermissionDenied(
                     'Must be a superuser or supply a nodegroup')

=== modified file 'src/provisioningserver/tags.py'
--- src/provisioningserver/tags.py	2012-10-05 14:16:54 +0000
+++ src/provisioningserver/tags.py	2012-10-08 15:06:24 +0000
@@ -83,7 +83,7 @@
     """
     path = '/api/1.0/nodegroups/%s/' % (nodegroup_uuid,)
     return process_response(client.post(
-        path, op='node_hardware_details', system_ids=system_ids))
+        path, op='node_hardware_details', as_json=True, system_ids=system_ids))
 
 
 def post_updated_nodes(client, tag_name, uuid, added, removed):
@@ -101,7 +101,8 @@
     task_logger.debug('Updating nodes for %s %s, adding %s removing %s'
         % (tag_name, uuid, len(added), len(removed)))
     return process_response(client.post(
-        path, op='update_nodes', nodegroup=uuid, add=added, remove=removed))
+        path, op='update_nodes', as_json=True,
+        nodegroup=uuid, add=added, remove=removed))
 
 
 def process_batch(xpath, hardware_details):