celery / django-celery-beat

Celery Periodic Tasks backed by the Django ORM
Other
1.69k stars 429 forks source link

Periodic tasks admin calls django.setup on first load #463

Open AlbericTrancart opened 2 years ago

AlbericTrancart commented 2 years ago

Summary:

When loading the "add periodic task page", the admin lists all available tasks and as a consequence calls django.setup() and seems an unintended side effect.

Exact steps to reproduce the issue:

  1. Load the "Add periodic tasks" page

Detailed information

We had an issue in production where a request to this page would never end (infinite load time until an eventual timeout). It happened because of a bug in another lib that didn't handle properly django.setup() being called during a request, which lead to this request being stuck.

What happens is that the admin loads a filter that calls the Celery loader. The property is cached but it has to run on first load. Then the loader sends a signal A django fixup listen on this signal and calls django.setup(), which then does a loooot of things (including rebooting log handlers, revalidating all models and admins...)

The TaskSelectWidget calling self.celery_app.loader.import_default_modules() to list available tasks works as is. I was wondering if this side effect was intended or desirable. It seems to me it is wanted on initialization but a request to the admin shouldn't revalidate everything.

I'm sorry as I don't know the celery source code well I don't have an alternative in mind but I wanted to raise the issue and see if it was intended or not.

Thanks!

auvipy commented 2 years ago

The TaskSelectWidget calling self.celery_app.loader.import_default_modules() to list available tasks works as is. I was wondering if this side effect was intended or desirable. It seems to me it is wanted on initialization but a request to the admin shouldn't revalidate everything.

I think your point is quite valid

AlbericTrancart commented 2 years ago

Maybe one way to do it would be to disconnect the receiver on the signal when using self.celery_app.loader.import_default_modules()?

Also, I checked and this line is also used in the run tasks action In this action, is there any reason why we couldn't use import_string(task.task) directly instead of self.celery_app.tasks.get(task.task)? (I checked and it worked in my case but there may be side effects that I'm not aware of)

auvipy commented 2 years ago

can you open a PR please?