Resolve #738 by requiring positional params to be significant

spotify · Feb 11, 2015 · cee6942 · cee6942
1 parent 11c69ac
commit cee6942
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 4 deletions.
diff --git a/luigi/task.py b/luigi/task.py
@@ -326,7 +326,7 @@ def get_param_values(cls, params, args, kwargs):
         exc_desc = '%s[args=%s, kwargs=%s]' % (task_name, args, kwargs)
 
         # Fill in the positional arguments
-        positional_params = [(n, p) for n, p in params]
+        positional_params = [(n, p) for n, p in params if p.significant]
         for i, arg in enumerate(args):
             if i >= len(positional_params):
                 raise parameter.UnknownParameterException('%s: takes at most %d parameters (%d given)' % (exc_desc, len(positional_params), len(args)))
@@ -341,8 +341,10 @@ def get_param_values(cls, params, args, kwargs):
                 raise parameter.UnknownParameterException('%s: unknown parameter %s' % (exc_desc, param_name))
             result[param_name] = arg
 
+        print task_name, ':'
         # Then use the defaults for anything not filled in
         for param_name, param_obj in params:
+            print param_name, param_obj
             if param_name not in result:
                 if not param_obj.has_task_value(task_name, param_name):
                     raise parameter.MissingParameterException("%s: requires the '%s' parameter to be set" % (exc_desc, param_name))

diff --git a/test/hadoop_test.py b/test/hadoop_test.py
@@ -155,6 +155,10 @@ def output(self):
         return self.get_output('failing')
 
 
+class MyStreamingJob(luigi.hadoop.JobTask):
+    param = luigi.Parameter()
+
+
 def read_wordcount_output(p):
     count = {}
     for line in p.open('r'):
@@ -228,6 +232,10 @@ def test_unicode_job(self):
     def test_failing_job(self):
         CommonTests.test_failing_job(self)
 
+    def test_instantiate_job(self):
+        # See https://github.com/spotify/luigi/issues/738
+        MyStreamingJob('param_value')
+
     def setUp(self):
         MockFile.fs.clear()
 

diff --git a/test/parameter_test.py b/test/parameter_test.py
@@ -273,11 +273,16 @@ def test_global_param_shared(self):
 
     def test_insignificant_parameter(self):
         class InsignificantParameterTask(luigi.Task):
-            foo = luigi.Parameter(significant=False)
+            foo = luigi.Parameter(significant=False, default='foo_default')
             bar = luigi.Parameter()
 
-        t = InsignificantParameterTask(foo='x', bar='y')
-        self.assertEqual(t.task_id, 'InsignificantParameterTask(bar=y)')
+        t1 = InsignificantParameterTask(foo='x', bar='y')
+        self.assertEqual(t1.task_id, 'InsignificantParameterTask(bar=y)')
+
+        t2 = InsignificantParameterTask('z')
+        self.assertEqual(t2.foo, 'foo_default')
+        self.assertEqual(t2.bar, 'z')
+        self.assertEqual(t2.task_id, 'InsignificantParameterTask(bar=z)')
 
 
 class TestNewStyleGlobalParameters(unittest.TestCase):