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

added optional celery support #266

Merged
merged 3 commits into from
Jul 13, 2015
Merged

added optional celery support #266

merged 3 commits into from
Jul 13, 2015

Conversation

jayfk
Copy link
Collaborator

@jayfk jayfk commented Jul 9, 2015

Uses the django:// BROKER_URL as default. One could argue that this might not be the best choice, because the current status is experimental <http://celery.readthedocs.org/en/latest/getting-started/brokers/django.html>_
. On the other hand, this is the only broker supported out of the box.

Any thoughts?

@pydanny
Copy link
Member

pydanny commented Jul 9, 2015

While this is good, I've never been a fan of sticking Celery code into the __init__.py. Fortunately, there's another way to do things in modern Django: https://www.reddit.com/r/django/comments/3cjtu4/asynchronous_tasks_with_django_and_celery/cswllql

Would you be up for using that pattern instead?

@jayfk
Copy link
Collaborator Author

jayfk commented Jul 9, 2015

Yeah, that looks promising, I wasn't aware of that. I'll try to put something together tomorrow.

From a first quick glance I see two possible problems:

  • Naming the app: Taskman sounds funny, but might confuses and isn't really obvious. Ideas?
  • If a user chooses to not install celery, he'll have an empty folder in his project. One way to circumvent that would be to put celery.py into the conf/ folder. I'm not entirely sure if that is possible. Since celery.py can also contain tasks, we'd end up with code in the conf/ folder.

@burhan
Copy link
Collaborator

burhan commented Jul 9, 2015

Regarding tasks, the best practice is to have a tasks.py in your app folder, this is also how celery discovers tasks; as per the documentation.

The only exception is tasks that are not tied to applications.

Taskman sounds good to me, to be honest.

@pydanny
Copy link
Member

pydanny commented Jul 13, 2015

I'm not a giant fan of taskman for a app name, but everything about this suffices. FWIW, I'm not a fan of putting code into __init__.py, hence why I prefer this approach.

If this approach turns out to have problems, well then there is a reason why we consider cookiecutter-django bleeding edge. 😜

pydanny added a commit that referenced this pull request Jul 13, 2015
added optional celery support
@pydanny pydanny merged commit f038f8f into cookiecutter:master Jul 13, 2015
This was referenced Jul 13, 2015
@pydanny
Copy link
Member

pydanny commented Jul 13, 2015

I just changed taskman to taskapp. 😄

@mwdiers
Copy link

mwdiers commented Jul 15, 2015

Hey, this is great, guys!

To restate here what I just posted on the Reddit:

I am using the celeryd init.d script provided by Celery and am not using force=True. This is the script in extra/generic-init.d/ I also follow the example in the docs.

I have not had any issues with autodiscovery. @sharedtask has worked perfectly for me, so I'm not sure what to tell you. There is no magic going on here. The line: app.autodiscover_tasks(lambda: settings.INSTALLED_APPS) should take care of this. It is supposed to check every app in settings.

@mwdiers
Copy link

mwdiers commented Jul 15, 2015

Also, if there are project-level tasks, then I think tasksapp/tasks.py is the right place to put them, and not tasksapp/celery.py

That way, autodiscovery will pick them up properly.

@mwdiers
Copy link

mwdiers commented Jul 15, 2015

One more note. I do not like the fact that one's settings module needs to be hard-coded into the celery config.

To work around this, I have a /config/celery_config.py. It contains just one line:

DJANGO_SETTINGS_MODULE = 'config.settings.production'

Then in my tasksapp/celery.py it have:

from config import celery_config

if not settings.configured:
    os.environ.setdefault('DJANGO_SETTINGS_MODULE', celery_config.DJANGO_SETTINGS_MODULE)

This keeps local instance configuration is the right location.

@Rustem
Copy link

Rustem commented May 19, 2017

Then you couldn't use @shared_task decorator, which is usually more appropriate unless u r building a tiny app. Even if you will be able to use it by defining an import statement from .taskapp.celery import app in the proj/__init__.py, I think it just makes things much more compicated, rather than clean. celery.py is an entry point for celery app, nothing else, then lets maintain it same way as we do with manage.py.

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

Successfully merging this pull request may close these issues.

None yet

5 participants