Skip to content

Commit

Permalink
Removing dead code.
Browse files Browse the repository at this point in the history
Note first, in passing, that `isinstance(type(action), type)` is a tautology. But, that aside, what this code seems to be trying to do is check whether the `action` is either a class representing a subclass of Task  or a function of either zero or one arguments. In the latter case it wants to make a new subclass of Task that invokes the function when executed. However, the function wrapping code can't possibly work unless I'm deeply missing something: In the `__init__` method of `FuncTask` the args parameter containing the positional arguments to `__init__` has to be either empty or contain one argument. However TaskBase in task.py defines an `__init__` that takes two positional arguments.

So any attempt to construct an instance of `FuncTask` would fail: if passed zero or one arguments, the call to `super(FuncTask, self).__init__(...)` will fail because the super constructer needs two positional arguments and if called with more than one arguments it will raise an `AssertionError` itself.

It's possible that the code was meant to refer to the args in the enclosing scope but, unless I'm very confused about Python, it does not so this code must never be used so might as well be deleted.

Testing Done:
Travis-CI.

Bugs closed: 1295

Reviewed at https://rbcommons.com/s/twitter/r/1960/
  • Loading branch information
gigamonkey authored and ity committed Mar 21, 2015
1 parent d828ece commit 1ccd3ee
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 30 deletions.
4 changes: 3 additions & 1 deletion src/python/pants/backend/core/tasks/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ def __init__(self, *args, **kwargs):
super(MyTask, self).__init__(*args, **kwargs)
...
This allows us to change Task.__init__()'s arguments without changing every subclass.
This allows us to change Task.__init__()'s arguments without
changing every subclass. If the subclass does not need its own
initialization, this method can (and should) be omitted entirely.
"""
self.context = context
self._workdir = workdir
Expand Down
27 changes: 1 addition & 26 deletions src/python/pants/goal/task_registrar.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,7 @@ def __init__(self, name, action, dependencies=None, serialize=True):
"""
self.serialize = serialize
self.name = name

if isinstance(type(action), type) and issubclass(action, Task):
self._task = action
else:
args, varargs, keywords, defaults = inspect.getargspec(action)
if varargs or keywords or defaults:
raise GoalError('Invalid action supplied, cannot accept varargs, keywords or defaults')
if len(args) > 1:
raise GoalError('Invalid action supplied, must accept either no args or else a single '
'Context object')

class FuncTask(Task):
def __init__(self, *args, **kwargs):
super(FuncTask, self).__init__(*args, **kwargs)

if not args:
self.action = action
elif len(args) == 1:
self.action = functools.partial(action, self.context)
else:
raise AssertionError('Unexpected fallthrough')

def execute(self):
self.action()

self._task = FuncTask
self._task = action

if dependencies:
# TODO(John Sirois): kill this warning and the kwarg after a deprecation cycle.
Expand Down
9 changes: 7 additions & 2 deletions tests/python/pants_test/base/test_extension_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from pkg_resources import (Distribution, EmptyProvider, VersionConflict, WorkingSet, working_set,
yield_lines)

from pants.backend.core.tasks.task import Task
from pants.base.build_configuration import BuildConfiguration
from pants.base.build_file_aliases import BuildFileAliases
from pants.base.exceptions import BuildConfigurationError
Expand All @@ -38,6 +39,10 @@ def get_metadata_lines(self, name):
return yield_lines(self.get_metadata(name))


class DummyTask(Task):
def execute(self): return 42


class LoaderTest(unittest.TestCase):
def setUp(self):
self.build_configuration = BuildConfiguration()
Expand Down Expand Up @@ -95,7 +100,7 @@ def test_load_valid_partial_aliases(self):

def test_load_valid_partial_goals(self):
def register_goals():
Goal.by_name('jack').install(TaskRegistrar('jill', lambda: 42))
Goal.by_name('jack').install(TaskRegistrar('jill', DummyTask))

with self.create_register(register_goals=register_goals) as backend_package:
Goal.clear()
Expand Down Expand Up @@ -202,7 +207,7 @@ def test_plugin_load_and_order(self):

def test_plugin_installs_goal(self):
def reg_goal():
Goal.by_name('plugindemo').install(TaskRegistrar('foo', lambda: 1))
Goal.by_name('plugindemo').install(TaskRegistrar('foo', DummyTask))
self.working_set.add(self.get_mock_plugin('regdemo', '0.0.1', reg=reg_goal))

# Start without the custom goal.
Expand Down
7 changes: 6 additions & 1 deletion tests/python/pants_test/tasks/test_reflect.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@

from pants.backend.core.register import build_file_aliases as register_core
from pants.backend.core.tasks import reflect
from pants.backend.core.tasks.task import Task
from pants.backend.jvm.register import build_file_aliases as register_jvm
from pants.backend.python.register import build_file_aliases as register_python
from pants.goal.goal import Goal
from pants.goal.task_registrar import TaskRegistrar
from pants_test.tasks.test_base import BaseTest


class DummyTask(Task):
def execute(self): return 42


class BuildsymsSanityTests(BaseTest):
@property
def alias_groups(self):
Expand All @@ -39,7 +44,7 @@ def test_java_library(self):
class GoalDataTest(BaseTest):
def test_gen_tasks_options_reference_data(self):
# can we run our reflection-y goal code without crashing? would be nice
Goal.by_name('jack').install(TaskRegistrar('jill', lambda: 42))
Goal.by_name('jack').install(TaskRegistrar('jill', DummyTask))
oref_data = reflect.gen_tasks_options_reference_data()
self.assertTrue(len(oref_data) > 0,
'Tried to generate data for options reference, got emptiness')

0 comments on commit 1ccd3ee

Please sign in to comment.