Unwrap task function if wrapped

marginalhours commented 3 years ago

What is the current behavior?

I made a small example of the Task API and noticed this weird behaviour: If the process where I was running queue.run() from didn't explicitly import the @queue.task-decorated function (even if unused), then instead of executing tasks instead it would endlessly queue new tasks.

This is because (I think) the wrapper gets registered instead of the function, so invoking it actually queues up a new task instead of invoking the task that's there.

What is the new behavior?

When storing a function in the registry, the hasattr check means that the original function (at least, one level of the original function - not sure about stacked decorators, could use a while: loop?) will be used instead of the wrapper. This fixes the problem locally.

Checklist

Please make sure the following requirements are complete:

[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
[x] All automated checks pass (CI/CD)

^ will work on the above now but since it's a small patch I wanted to get this open to get people's thoughts

Thanks,

Tom

stas commented 3 years ago

Hmm, @MisterKeefe it would be amazing if you could provide an example how to reproduce this.

It never happened to me. Thank you!

marginalhours commented 3 years ago

@stas Sure, here you go: https://github.com/MisterKeefe/pq-issue-replication

Let me know if any issues running it! Quite possible it's a weirdness with the way I've set things up :D

malthe commented 3 years ago

@stas would be nice to have a test on this behavior I think.

malthe / pq