Merge "Support for optional task arguments"

taskflow/task.py

@@ -78,34 +78,33 @@ def _build_rebind_dict(args, rebind_args):
         raise TypeError('Invalid rebind value: %s' % rebind_args)
 
 
-def _check_args_mapping(task_name, rebind, args, accepts_kwargs):
-    args = set(args)
-    rebind = set(rebind.keys())
-    extra_args = rebind - args
-    missing_args = args - rebind
-    if not accepts_kwargs and extra_args:
-        raise ValueError('Extra arguments given to task %s: %s'
-                         % (task_name, sorted(extra_args)))
-    if missing_args:
-        raise ValueError('Missing arguments for task %s: %s'
-                         % (task_name, sorted(missing_args)))
-
-
 def _build_arg_mapping(task_name, reqs, rebind_args, function, do_infer):
     """Given a function, its requirements and a rebind mapping this helper
     function will build the correct argument mapping for the given function as
     well as verify that the final argument mapping does not have missing or
     extra arguments (where applicable).
     """
-    task_args = reflection.get_required_callable_args(function)
-    accepts_kwargs = reflection.accepts_kwargs(function)
+    task_args = reflection.get_callable_args(function, required_only=True)
     result = {}
     if reqs:
         result.update((a, a) for a in reqs)
     if do_infer:
         result.update((a, a) for a in task_args)
     result.update(_build_rebind_dict(task_args, rebind_args))
-    _check_args_mapping(task_name, result, task_args, accepts_kwargs)
+
+    if not reflection.accepts_kwargs(function):
+        all_args = reflection.get_callable_args(function, required_only=False)
+        extra_args = set(result) - set(all_args)
+        if extra_args:
+            extra_args_str = ', '.join(sorted(extra_args))
+            raise ValueError('Extra arguments given to task %s: %s'
+                             % (task_name, extra_args_str))
+
+    # NOTE(imelnikov): don't use set to preserve order in error message
+    missing_args = [arg for arg in task_args if arg not in result]
+    if missing_args:
+        raise ValueError('Missing arguments for task %s: %s'
+                         % (task_name, ' ,'.join(missing_args)))
     return result
 
 

taskflow/tests/unit/test_task.py

@@ -31,6 +31,11 @@ def execute(self, spam, **kwargs):
         pass
 
 
+class DefaultArgTask(task.Task):
+    def execute(self, spam, eggs=()):
+        pass
+
+
 class DefaultProvidesTask(task.Task):
     default_provides = 'def'
 
@@ -98,6 +103,14 @@ def test_requires_explicit_not_enough(self):
         with self.assertRaisesRegexp(ValueError, '^Missing arguments'):
             MyTask(auto_extract=False, requires=('spam', 'eggs'))
 
+    def test_requires_ignores_optional(self):
+        my_task = DefaultArgTask()
+        self.assertEquals(my_task.requires, set(['spam']))
+
+    def test_requires_allows_optional(self):
+        my_task = DefaultArgTask(requires=('spam', 'eggs'))
+        self.assertEquals(my_task.requires, set(['spam', 'eggs']))
+
     def test_rebind_all_args(self):
         my_task = MyTask(rebind={'spam': 'a', 'eggs': 'b', 'context': 'c'})
         self.assertEquals(my_task.rebind, {

taskflow/tests/unit/test_utils.py

@@ -99,41 +99,46 @@ def test_callable_class_call(self):
                                           '__call__')))
 
 
-class GetRequiredCallableArgsTest(test.TestCase):
+class GetCallableArgsTest(test.TestCase):
 
     def test_mere_function(self):
-        result = reflection.get_required_callable_args(mere_function)
+        result = reflection.get_callable_args(mere_function)
         self.assertEquals(['a', 'b'], result)
 
     def test_function_with_defaults(self):
-        result = reflection.get_required_callable_args(function_with_defs)
+        result = reflection.get_callable_args(function_with_defs)
+        self.assertEquals(['a', 'b', 'optional'], result)
+
+    def test_required_only(self):
+        result = reflection.get_callable_args(function_with_defs,
+                                              required_only=True)
         self.assertEquals(['a', 'b'], result)
 
     def test_method(self):
-        result = reflection.get_required_callable_args(Class.method)
+        result = reflection.get_callable_args(Class.method)
         self.assertEquals(['self', 'c', 'd'], result)
 
     def test_instance_method(self):
-        result = reflection.get_required_callable_args(Class().method)
+        result = reflection.get_callable_args(Class().method)
         self.assertEquals(['c', 'd'], result)
 
     def test_class_method(self):
-        result = reflection.get_required_callable_args(Class.class_method)
+        result = reflection.get_callable_args(Class.class_method)
         self.assertEquals(['g', 'h'], result)
 
     def test_class_constructor(self):
-        result = reflection.get_required_callable_args(ClassWithInit)
+        result = reflection.get_callable_args(ClassWithInit)
         self.assertEquals(['k', 'l'], result)
 
     def test_class_with_call(self):
-        result = reflection.get_required_callable_args(CallableClass())
+        result = reflection.get_callable_args(CallableClass())
         self.assertEquals(['i', 'j'], result)
 
     def test_decorators_work(self):
         @lock_utils.locked
         def locked_fun(x, y):
             pass
-        result = reflection.get_required_callable_args(locked_fun)
+        result = reflection.get_callable_args(locked_fun)
         self.assertEquals(['x', 'y'], result)
 
 

taskflow/utils/reflection.py

@@ -111,11 +111,18 @@ def _get_arg_spec(function):
     return inspect.getargspec(function), bound
 
 
-def get_required_callable_args(function):
-    """Get names of argument required by callable"""
+def get_callable_args(function, required_only=False):
+    """Get names of callable arguments
+
+    Special arguments (like *args and **kwargs) are not included into
+    output.
+
+    If required_only is True, optional arguments (with default values)
+    are not included into output.
+    """
     argspec, bound = _get_arg_spec(function)
     f_args = argspec.args
-    if argspec.defaults:
+    if required_only and argspec.defaults:
         f_args = f_args[:-len(argspec.defaults)]
     if bound:
         f_args = f_args[1:]