operations: smarter handling of call and generated arguments

_operation methods now only need to accept those generated arguments that are
actually used.

circle/common/operations.py

+from inspect import getargspec
 from logging import getLogger
 
 from .models import activity_context
@@ -29,23 +30,51 @@ class Operation(object):
     def __prelude(self, kwargs):
         """This method contains the shared prelude of call and async.
         """
-        skip_auth_check = kwargs.setdefault('system', False)
-        user = kwargs.setdefault('user', None)
-        parent_activity = kwargs.pop('parent_activity', None)
+        defaults = {'parent_activity': None, 'system': False, 'user': None}
+
+        allargs = dict(defaults, **kwargs)  # all arguments
+        auxargs = allargs.copy()  # auxiliary (i.e. only for _operation) args
+        # NOTE: consumed items should be removed from auxargs, and no new items
+        # should be added to it
+
+        skip_auth_check = auxargs.pop('system')
+        user = auxargs.pop('user')
+        parent_activity = auxargs.pop('parent_activity')
+
+        # check for unexpected keyword arguments
+        argspec = getargspec(self._operation)
+        if argspec.keywords is None:  # _operation doesn't take ** args
+            unexpected_kwargs = set(arg for arg in auxargs
+                                    if arg not in argspec.args)
+            if unexpected_kwargs:
+                raise TypeError("Operation got unexpected keyword arguments: "
+                                "%s" % ", ".join(unexpected_kwargs))
 
         if not skip_auth_check:
             self.check_auth(user)
         self.check_precond()
-        return self.create_activity(parent=parent_activity, user=user)
 
-    def _exec_op(self, activity, user, **kwargs):
+        activity = self.create_activity(parent=parent_activity, user=user)
+
+        return activity, allargs, auxargs
+
+    def _exec_op(self, allargs, auxargs):
         """Execute the operation inside the specified activity's context.
         """
-        with activity_context(activity, on_abort=self.on_abort,
+        # compile arguments for _operation
+        argspec = getargspec(self._operation)
+        if argspec.keywords is not None:  # _operation takes ** args
+            arguments = allargs.copy()
+        else:  # _operation doesn't take ** args
+            arguments = {k: v for (k, v) in allargs.iteritems()
+                         if k in argspec.args}
+        arguments.update(auxargs)
+
+        with activity_context(allargs['activity'], on_abort=self.on_abort,
                               on_commit=self.on_commit):
-            return self._operation(activity=activity, user=user, **kwargs)
+            return self._operation(**arguments)
 
-    def _operation(self, activity, user, system, **kwargs):
+    def _operation(self, **kwargs):
         """This method is the operation's particular implementation.
 
         Deriving classes should implement this method.
@@ -65,11 +94,9 @@ class Operation(object):
         logger.info("%s called asynchronously on %s with the following "
                     "parameters: %r", self.__class__.__name__, self.subject,
                     kwargs)
-        activity = self.__prelude(kwargs)
-        return self.async_operation.apply_async(args=(self.id,
-                                                      self.subject.pk,
-                                                      activity.pk),
-                                                kwargs=kwargs,
+        activity, allargs, auxargs = self.__prelude(kwargs)
+        return self.async_operation.apply_async(
+            args=(self.id, self.subject.pk, activity.pk, allargs, auxargs, ),
             queue=self.async_queue)
 
     def call(self, **kwargs):
@@ -88,8 +115,9 @@ class Operation(object):
         logger.info("%s called (synchronously) on %s with the following "
                     "parameters: %r", self.__class__.__name__, self.subject,
                     kwargs)
-        activity = self.__prelude(kwargs)
-        return self._exec_op(activity=activity, **kwargs)
+        activity, allargs, auxargs = self.__prelude(kwargs)
+        allargs['activity'] = activity
+        return self._exec_op(allargs, auxargs)
 
     def check_precond(self):
         pass

circle/vm/tasks/local_tasks.py

@@ -2,7 +2,8 @@ from manager.mancelery import celery
 
 
 @celery.task
-def async_instance_operation(operation_id, instance_pk, activity_pk, **kwargs):
+def async_instance_operation(operation_id, instance_pk, activity_pk, allargs,
+                             auxargs):
     from vm.models import Instance, InstanceActivity
     instance = Instance.objects.get(pk=instance_pk)
     operation = getattr(instance, operation_id)
@@ -12,11 +13,13 @@ def async_instance_operation(operation_id, instance_pk, activity_pk, **kwargs):
     activity.task_uuid = async_instance_operation.request.id
     activity.save()
 
-    return operation._exec_op(activity=activity, **kwargs)
+    allargs['activity'] = activity
+
+    return operation._exec_op(allargs, auxargs)
 
 
 @celery.task
-def async_node_operation(operation_id, node_pk, activity_pk, **kwargs):
+def async_node_operation(operation_id, node_pk, activity_pk, allargs, auxargs):
     from vm.models import Node, NodeActivity
     node = Node.objects.get(pk=node_pk)
     operation = getattr(node, operation_id)
@@ -26,4 +29,6 @@ def async_node_operation(operation_id, node_pk, activity_pk, **kwargs):
     activity.task_uuid = async_node_operation.request.id
     activity.save()
 
-    return operation._exec_op(activity=activity, **kwargs)
+    allargs['activity'] = activity
+
+    return operation._exec_op(allargs, auxargs)