Cookiecutter-django: Changes on celery implementation

[cdvv7788]

According to celery docs:

Note that this example project layout is suitable for larger projects, for simple projects you may use a single contained module that defines both the app and tasks, like in the First Steps with Celery tutorial.

With that in mind, the fact that the celery setup file is inside of a folder in the path where we hold our apps is a little misleading. It is configured so it discovers tasks with the @shared_task decorator inside of any app, but it is inside of a folder named taskapp, again, misleading.

Another thing that i would like to point is that celery.py is a setup file, where we create the instance for the celery application. In that way it is very similar to the wsgi.py file, where we setup the wsgi app. Different things, i know, but with the same purpose.

Following that line of thought i would like to propose removing the taskapp and move the celery.py file (and the changes to init.py) to the config folder. Simpler structure and a more coherent setup in my opinion.

[pydanny]

As I'm a huge fan of simplicity, I like your suggestion. That said, let's go query the best person in the world when it comes to Celery and see what they have to say.

Hey @ask, do you want to say anything about having Cookiecutter Django re-architect it's Celery implementation?

[jayfk]

Here's some context on why we choose to implement this differently: https://github.com/pydanny/cookiecutter-django/pull/266

[cdvv7788]

From what i see, the options are:

• Keep an app for celery and use the init file.
• Keep an app for celery and initialize it in the AppConfig (Current method used)
• Move celery out of an app and use an init file.

The proposed change would have the following consequences:

• More content on the init file of the config module (which @pydanny seems to dislike according to #266)
• One less app (which is good, we are not having a whole app for a single file)
• We could start celery with: celery -A config, which is shorter :P
• Concerns are separated in a clear way (imho). Config module would be in charge of setting up the project (wsgi, celery, etc).
• We would lose a way to have all our tasks centralized (which could be good or bad, depending on your current setup, but all tasks should be inside the app related to the functionality anyway, not in a single app...autodiscover rules).

Anyway, thanks for checking the suggestion.

[ask]

Thanks for inviting me!

I think there are two very important things to consider here:

1) The @shared_task decorator means you don't need to import an explicit app instance
to define a task.

Good for reusable apps, for example. django-rest-framework could define tasks without knowing
where you define the app.

2) If @shared_task is used, the app needs to be created as early as possible when Django starts

The tutorial adds this to proj/__init__.py to make sure that happens:

# proj/__init__.py
from .celery import app as celery_app

If you always pass explicit app instances around, you don't have to worry about this part
but you also lose the ability for reusable apps to add tasks.

The reusable apps could provide with some setup code that you add:

app = Celery()
rest_framework.add_tasks(app)

That's definitely more correct, and robust, less magic and all that, but it's also less convenient.

Technically you could use @shared_task with both approaches: defining the app in the project, or in a self-contained reusable app, but you still need to make the app is created before you call tasks
in views and so on, which means you would need:

# proj/_init__.py
from tasks import app as celery_app

[matthewslaney]

I was reading through how you are implementing this because, like @pydanny, I didn't like the idea of putting Celery code into __init__.py. Creating an app that can be added to INSTALLED_APPS is exactly how I was thinking to implement it, so it's great to see an example of that being done. Reading through this and the other related Issues/PRs, I had a couple questions and a comment:

1. Per what @ask said, is there a concern that @shared_task would be called in a view before the AppConfig would be loaded by django.setup()? I thought that the app registry would be fully set up sufficiently early to avoid that.
2. In the current implementation, it's adding {{cookiecutter.project_slug}}.taskapp.celery.CeleryConfig to INSTALLED_APPS. Is there a reason for that extended, explicit call rather than putting an apps.py file in taskapp and just putting a simple taskapp in the INSTALLED_APPS list?

Comment: I noticed in the reddit post linked in #266 that inspired this method that it states that some of this code is put in the .ready() method of the AppConfig to "ensure that the app's setup will be called once, and only once, when Django starts." However, the Django docs state that there are edge cases where .ready() is called more than once. Is that a concern?

[Palisand]

@matthewslaney Brings up some good points. Can anyone answer his questions?

[browniebroke]

1. My understanding is that there is a case where if an app (yours or 3rd party like DRF) declaring a task is installed before the project's Celery app is initialised, then Celery might not find it.

2. This is how Django recommends to do it... It has nothing to do with the module name being apps.py. The short form is for Django version that pre-dates 1.7, and it can be mapped using the default_app_config attribute at the module level. It's not recommended though:

   New applications should avoid default_app_config. Instead they should require the dotted path to the appropriate AppConfig subclass to be configured explicitly in INSTALLED_APPS.

To go back to the main topic of this issue, I'm in favor of building the Celery app outside of a Django app, I'm on the side of the "it's more like the wsgi.py" argument.

The tasks itself should be provided by each Django app, not by the Celery app (which is not a Django app IMO) and I don't think that centralising them is a good thing.

[browniebroke]

I've started a pull request to implement what has been discussed here: https://github.com/pydanny/cookiecutter-django/pull/2016

I've also added a dummy task to the user app, its main purpose is to:

• demonstrate how tasks should be defined by Django apps
• demonstrate how they should be tested
• help us test upcoming Celery upgrades

It's pretty small, but any help in getting this reviewed & testing is welcome.