Skip to content

Use new Celery, use new application pattern #3237

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

Merged
merged 11 commits into from
Nov 11, 2017
Merged

Conversation

agjohnson
Copy link
Contributor

  • Use modern celery
  • Drop djcelery
  • New pattern for starting celery
  • Bump redis to 2.10.6 to avoid startup bug, change autodiscover call

* Use modern celery
* Drop djcelery
* New pattern for starting celery
* Bump redis to 2.10.6 to avoid startup bug, change autodiscover call
@agjohnson agjohnson force-pushed the upgrade-celery-pattern branch from c82a9aa to b0c3ad5 Compare November 8, 2017 23:37
@@ -572,7 +569,7 @@ def update_imported_docs(version_pk):


# Web tasks
@task(queue='web')
@shared_task(queue='web')
Copy link
Member

@safwanrahman safwanrahman Nov 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont like using shared_task unless its a reusable app where you can not get the app.
You can import the app from worker then use the decorator task
something like

from readthedocs.worker import app

@app.task(queue='web')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I'll try the change.

@agjohnson
Copy link
Contributor Author

Think we've been through this PR a few ways now. I'll get this merged

@agjohnson agjohnson merged commit 96b23c8 into master Nov 11, 2017
@humitos humitos mentioned this pull request Nov 25, 2017
@stsewd stsewd deleted the upgrade-celery-pattern branch August 15, 2018 22:14
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.

3 participants