Use task variable name when task name is undefined. #766

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

In fabric.main.extract_tasks(), the dict mapping names to new-style task
objects now uses the variable name of the object when the object's name
attribute is not defined. (Technically, when not obj.name or obj.name
== 'undefined').

This change brings the behavior for mapping tasks to names in compliance
with the fabric documentation.
http://docs.fabfile.org/en/1.4.3/usage/tasks.html#new-style-tasks:

Subclass Task (Task itself is intended to be abstract), define a run
method, and instantiate your subclass at module level.  Instances’
name attributes are used as the task name; if omitted the instance’s
variable name will be used instead.

A test case was added to confirm that the task name equal the variable
name when the task object name attribute is undefined. This test fails
with the old code and passes with the patched code.

This is my first fabric patch. I did my best to adhere to the guidelines at http://docs.fabfile.org/en/1.4.3/development.html. Please let me know if I did anything wrong and I'll try to fix it.

Todd DeLuca added some commits Oct 28, 2012

Todd DeLuca Use task variable name when task name is undefined.
In fabric.main.extract_tasks(), the dict mapping names to new-style task
objects now uses the variable name of the object when the object's name
attribute is not defined.  (Technically, when not obj.name or obj.name
== 'undefined').

This change brings the behavior for mapping tasks to names in compliance
with the fabric documentation.
http://docs.fabfile.org/en/1.4.3/usage/tasks.html#new-style-tasks:

    Subclass Task (Task itself is intended to be abstract), define a run
    method, and instantiate your subclass at module level.  Instances’
    name attributes are used as the task name; if omitted the instance’s
    variable name will be used instead.

A test case was added to confirm that the task name equal the variable
name when the task object name attribute is undefined.  This test fails
with the old code and passes with the patched code.
bd9e038
Todd DeLuca Added issue #766 note to the changelog. a79ede3
Todd DeLuca Issue-766 Add name attribute to tasks without one.
New-style Task subclass objects which do not define a name attribute end
up with the default obj.name = 'undefined' from Task.  When the tasks
are extracted from a loaded fabfile, obj.name should be set to the name
of the instance variable, since obj.name is used internally by fabric in
some circumstances, e.g. when `execute()` is called.  This commit sets
`obj.name = name` for task objects without a name, or with an undefined
name.
a45d9d5

The fix to set the task name on the command line from the instance variable name reveals another bug, I think. When running an "instance" task from the command line, the output uses the name of the instance variable. When running an "instance" task from within another task (via execute), the output uses the instance's name attribute.

Here is a fabfile.py that replicates this behavior:

from fabric.tasks import Task
from fabric.api import *

env.hosts = ['localhost']

class MyTask(Task):
    def run(self):
        run('uname')

# Since task instance does not define 'name' attribute, it should be
# set to 'uname'
uname = MyTask()

@task
def all():
    execute(uname)

Here is the output of several command lines that demonstrates a few things:

  • the "instance" task is listed correctly and available to be run from the command line.
  • the name of the instance variable is used in the output when the task is run directly from the command line.
  • 'undefined' is used (the name attribute of the instance) when the task is run via execute from the all task.
td23@todd-mac:~$ fab -l
Available commands:

    all
    uname
td23@todd-mac:~$ fab uname
[localhost] Executing task 'uname'
[localhost] run: uname
[localhost] out: Darwin


Done.
Disconnecting from localhost... done.
td23@todd-mac:~$ fab all
[localhost] Executing task 'all'
[localhost] Executing task 'undefined'
[localhost] run: uname
[localhost] out: Darwin


Done.
Disconnecting from localhost... done.

I have fixed problem by setting obj.name = name in fabric.main.extract_tasks(). I added a test to make sure obj.name is set. Nose output before the fix shows the test failing as it should:

td23@todd-mac:~/proj/fabric$ nosetests tests/test_main.py
.F.................................................................
======================================================================
FAIL: A new-style tasks with undefined name attribute should use the instance
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/td23/proj/fabric/tests/test_main.py", line 433, in test_class_based_tasks_are_found_with_variable_name
    eq_(funcs['foo'].name, 'foo')
  File "/Users/td23/proj/fabric/tests/utils.py", line 248, in eq_
    assert result == expected, msg or default_msg
AssertionError: 
Expected:
foo

Got:
undefined


--------------------------------- aka -----------------------------------------

Expected:
'foo'

Got:
'undefined'


----------------------------------------------------------------------
Ran 67 tests in 0.081s

FAILED (failures=1)

Nose output after the fix shows the test succeeding as it should:

td23@todd-mac:~/proj/fabric$ nosetests tests/test_main.py
...................................................................
----------------------------------------------------------------------
Ran 67 tests in 0.039s

OK

@bitprophet bitprophet added a commit that referenced this pull request Dec 28, 2012

@bitprophet Todd DeLuca + bitprophet Added issue #766 note to the changelog. b602fe6
Owner

bitprophet commented Dec 28, 2012

I've rebased (a copy of) your branch against the latest bugfix line (1.5) and I acknowledge your fix for the issue mentioned above (re: lazily filling in empty task .name attributes with the variable name so things are consistent). Thanks!

EDIT: I guess rebasing was the wrong idea, Github can't tell the changes were pulled in. Meh :) My fault for letting things get behind. Closing.

bitprophet closed this Dec 28, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment