Dependency on celery lacks documentation

[sumpfralle]

Describe the bug

Recently (commit a6a21d3) a dependency on celery was introduced. Probably it was meant to be triggered only, if celery is active (which would cause the import of tasks.py). But in reality it is also triggered, if other task queue handlers are running (in my case: huey).

To Reproduce
Add oauth2_provider and huey to INSTALLED_APPS and ensure, that the celery module is not available.
python3 manage.py runserver will fail due to the missing celery module.

Expected behavior
I could imagine the following behaviors to be more desirable:

• A) a missing celery module is silently ignored
• B) celery is only imported, if celery is found in settings.INSTALLED_APPS
• C) the active task queue manager (e.g. celery or huey) is detected (e.g. by inspecting settings.INSTALLED_APPS) and the relevant decorator is selected

Version
1.7.0

• I have tested with the latest published release and it's still a problem.
• I have tested with the master branch and it's still a problem.

Additional context

[n2ygk]

Thanks for this report. Can you submit a PR which would fix this?

[sumpfralle]

To be honest, I cannot tell, what the original commit is supposed to do exactly.

I think, it is supposed to wrap the function in order to enforce its execution in a task queue.
But I have no idea, how the task is actually scheduled.

In huey there are two types of function decorators:

• db_task (no parameters): for tasks to be called explicitly by code from within the django application
• db_periodic_task (parameters specify the wanted period): for tasks, which should be executed periodically (they are not called explicitly)

The original commit, which introduced the celery dependency (a6a21d3) references the celery documentation for periodic tasks, but the shared_task decorator is not described there. Thus I guess, that the shared_task decorator is similar to the db_task decorator in huey (see above), i.e. it is not executed periodically.

In this case I do not understand the purpose of the current snippet. Maybe there is something missing?

@Natureshadow: maybe you could clarify, how exactly your addition should be used?

[MichaelVoelkel]

To unblock everyone trying to upgrade to 1.7.0, could you maybe just state celery as dependency in your setup.cfg? For now, we downgraded again because pip-compile will not find this dependency, so it breaks all libraries that depend on this lib or a lib that depends on it etc. (especially since issue is open for 25 days now)

[Natureshadow]

I think, it is supposed to wrap the function in order to enforce its execution in a task queue. But I have no idea, how the task is actually scheduled.

That depends on what scheduler you use with Celery.

The original commit, which introduced the celery dependency (a6a21d3) references the celery documentation for periodic tasks, but the shared_task decorator is not described there. Thus I guess, that the shared_task decorator is similar to the db_task decorator in huey (see above), i.e. it is not executed periodically.

https://docs.celeryproject.org/en/stable/django/first-steps-with-django.html#using-the-shared-task-decorator

In this case I do not understand the purpose of the current snippet. Maybe there is something missing?

I don't think so.

@Natureshadow: maybe you could clarify, how exactly your addition should be used?

This is described in the original PR and in the Celery documentation linked from there and from the DOT docs.

[sumpfralle]

Thank you for helping to solve this issue!

The original commit, which introduced the celery dependency (a6a21d3) references the celery documentation for periodic tasks, but the shared_task decorator is not described there. Thus I guess, that the shared_task decorator is similar to the db_task decorator in huey (see above), i.e. it is not executed periodically.

https://docs.celeryproject.org/en/stable/django/first-steps-with-django.html#using-the-shared-task-decorator

Thank you.
Sorry, I should have mentioned, that I read this part of the the documentation before.
It only contains the following explanation for shared_task:

The tasks you write will probably live in reusable apps, and reusable apps cannot depend on the project itself, so you also cannot import your app instance directly.

The @shared_task decorator lets you create tasks without having any concrete app instance:

My understanding of the above documentation is, that the clear_tokens function is just decorated for being executed via celery. But I am quite confident, that this does not involve any scheduling (e.g. "call this function every Friday at midnight"). Thus the user needs to arrange the execution of clear_tokens somehow (e.g. calling it explicitly or specifying an execution period).
Is this correct?

Sorry for me trying to be precise here, but even after going through your PR and the referenced parts of the celery documentation multiple times, I fail to grasp the exact effect of this decorated function. I hope, that your answer to my above question will clarify this for me.

Thank you!

[Natureshadow]

My understanding of the above documentation is, that the *clear_tokens* function is just decorated for being executed via celery. But I am quite confident, that this does not involve any /scheduling/ (e.g. "call this function every Friday at midnight"). Thus the user needs to arrange the execution of *clear_tokens* somehow (e.g. calling it explicitly or specifying an execution period). Is this correct? Sorry for me trying to be precise here, but even after going through your PR and the referenced parts of the celery documentation multiple times, I fail to grasp the /exact/ effect of this decorated function. I hope, that your answer to my above question will clarify this for me.

Yes, you need to use some scheduler and configure celery-beat to use it, or trigger the task in any other way.

[n2ygk]

@MichaelVoelkel said:

To unblock everyone trying to upgrade to 1.7.0, could you maybe just state celery as dependency in your setup.cfg? For now, we downgraded again because pip-compile will not find this dependency, so it breaks all libraries that depend on this lib or a lib that depends on it etc. (especially since issue is open for 25 days now)

My understanding is that this decorator does not add a dependency on celery and that it is only inadvertently a problem if you are using huey as described by @sumpfralle since it also looks for a tasks.py

So this is not blocking everyone from upgrading and is only a conflict for those using huey.

Meanwhile, a PR that resolves the conflict between huey and celery both autoloading tasks.py would be greatly appreciated by someone who is familiar with them. Perhaps a test in tasks.py to do a conditional import? Or perhaps a setting that avoids the autoloading confict?

[MichaelVoelkel]

My understanding is that this decorator does not add a dependency on celery and that it is only inadvertently a problem if you are using huey as described by @sumpfralle since it also looks for a tasks.py

Okay, sorry, but I do not get this. Within the codebase of this package, you import celery. For me, this is enough to say that the codebase depends on celery and therefore this dependency should be made explicit. Sure, if you do happen to not use tasks.py, the error might not show up. But well, the codebase as a matter of fact contains this file and therefore this dependency and as library creator you should expect people to also use it (if not, why not remove the file altogether?).

But is this really your quality approach? Do not mention dependencies that might not be used be a library user?