Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error on passing task to other task #376

Closed
Lense opened this issue Jul 14, 2016 · 6 comments
Closed

Error on passing task to other task #376

Lense opened this issue Jul 14, 2016 · 6 comments

Comments

@Lense
Copy link

Lense commented Jul 14, 2016

from invoke import task

@task
def preTask(ctx):
    print("preTask")

@task
def testTask(ctx, otherTask=preTask):
    print("otherTask @ ", otherTask)

spits out

Traceback (most recent call last):
  File "/usr/bin/inv", line 11, in <module>
    sys.exit(program.run())
  File "/usr/lib/python2.7/site-packages/invoke/program.py", line 269, in run
    self._parse(argv)
  File "/usr/lib/python2.7/site-packages/invoke/program.py", line 329, in _parse
    self.parse_tasks()
  File "/usr/lib/python2.7/site-packages/invoke/program.py", line 490, in parse_tasks
    self.parser = Parser(contexts=self.collection.to_contexts())
  File "/usr/lib/python2.7/site-packages/invoke/collection.py", line 325, in to_contexts
    name=primary, aliases=aliases, args=task.get_arguments()
  File "/usr/lib/python2.7/site-packages/invoke/tasks.py", line 205, in get_arguments
    new_arg = Argument(**self.arg_opts(name, default, taken_names))
  File "/usr/lib/python2.7/site-packages/invoke/tasks.py", line 180, in arg_opts
    if default not in (None, NO_DEFAULT):
  File "/usr/lib/python2.7/site-packages/invoke/tasks.py", line 83, in __eq__
    if self.name != other.name:
AttributeError: 'NoneType' object has no attribute 'name'

I was originally doing something really dumb because I didn't understand how invoke worked, but I came across this weird case. Feel free to close this.

@Lense Lense changed the title Improve handling of non-list pre Error on passing task to other task Jul 14, 2016
@bitprophet
Copy link
Member

bitprophet commented Jul 20, 2016

Huh, interesting, I think this is because the value you're passing in the signature of testTask is another Task object, and the argument examination stuff obviously didn't anticipate that. Thanks for the report!

Perhaps more deeply, __eq__ may not be well implemented for Task objects - it should say "nope, not equal" if the other object is None. 🤔 Ought to be a trivial fix!

@rsxm
Copy link

rsxm commented Aug 20, 2016

Hi, while testing/researching inspect.Signature for #357, I also came across this issue.

I'm still making my way through the codebase and don't yet fully understand why the eq method fails when a task is passed as an argument, but I've found two solutions that stops it from failing:

  1. Add type checking to eq

    if not isinstance(other, (Task, Call)):
        return False
    
  2. Return False on None and when 'name' attribute does not exist

if other is None:
    return False

if not getattr(other, 'name', None):
    return False

I suspect 1 is the better solution, but I don't know Invoke well enough to be sure. Either way, this is indeed "Low-hanging Fruit", and I've got a pull request-able branch (with updated tests) for both approaches.

However, I've got a suspicion that this only treats the symptom, and that the real cause is somehow related to the argspec/arg_opts logic.

rsxm added a commit to rsxm/invoke that referenced this issue Aug 20, 2016
Add type checking so that only a Task instances can potentially equal
another Task instance. Same logic apply to Call instances.

Fixes pyinvoke#376
@rsxm
Copy link

rsxm commented Aug 20, 2016

I've figured out how to properly debug Invoke tasks and located the root cause. Turns out there is nothing complicated about this issue :-)

Task.arg_opts simply tries to infer the type of the default value, but first it checks to make sure that the default is not None. If the default happens to be a Task instance, this check calls it's eq method, which breaks because it can't deal with anything else besides a Task instance (or more precisely, an object with a 'name' and a 'body' attribute).

I've opened a pull request that fixes this issue for your review.

@bitprophet
Copy link
Member

Thanks @jr-minnaar - will look that over when I get to wrapping up 0.12.3.

@bitprophet bitprophet modified the milestones: 0.18.x, Next bugfix Jul 12, 2018
@kuwv
Copy link
Contributor

kuwv commented Feb 4, 2023

#906

@bitprophet
Copy link
Member

Rolling into #906 which includes the fix and a changelog entry for this issue.

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

No branches or pull requests

4 participants