Update celery task discovery config. #25479

feanil · 2020-10-29T14:22:08Z

Update the celery startup code to more closely match the docs. We believe some of these imports are causeing circulare
dependencies that are causing race conditions in task discovery. Since all django apps should be loaded using the
django fixup in celery 4, we shouldn't need these other overrides.

https://github.com/celery/celery/blob/v4.4.7/celery/fixups/django.py
https://docs.celeryproject.org/en/v4.4.7/django/first-steps-with-django.html#using-celery-with-django

cms/envs/common.py

jmbowman

Just one suggestion for further cleanup, which can probably wait until after we determine if this even works.

cms/envs/common.py

Update the celery startup code to more closely match the docs. We believe some of these imports are causeing circulare dependencies that are causing race conditions in task discovery. Since all django apps should be loaded using the django fixup in celery 4, we shouldn't need these other overrides. https://github.com/celery/celery/blob/v4.4.7/celery/fixups/django.py https://docs.celeryproject.org/en/v4.4.7/django/first-steps-with-django.html#using-celery-with-django

feanil · 2020-10-29T14:26:55Z

Removed the config setting entirely as suggested.

cpennington · 2020-10-29T14:42:34Z

cms/celery.py

 import os

 from celery import Celery
-from django.conf import settings


I can't comment on the actual line, but probably change the default value for DJANGO_SETTINGS_MODULE in here too?

edx-status-bot · 2020-10-29T14:51:52Z

Your PR has finished running tests. There were no failures.

edx-pipeline-bot · 2020-10-29T16:00:44Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

edx-pipeline-bot · 2020-10-29T16:12:15Z

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

edx-pipeline-bot · 2020-10-29T16:26:39Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

edx-pipeline-bot · 2020-10-29T16:38:07Z

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

edx-pipeline-bot · 2020-10-29T19:27:30Z

EdX Release Notice: This PR has been deployed to the production environment.

pomegranited · 2020-11-28T06:42:38Z

Hi @feanil -- I'm running latest master on one of my sandboxes, but it's throwing UnregisteredTaskException for all the instructor_task.tasks and also openedx.core.djangoapps.heartbeat.tasks.sample_task, which we use for extended heartbeat checks.

Is there something I need to do to ensure these tasks are auto-discovered by celery?

CC @jmbowman @ormsbee

feanil · 2020-11-30T21:42:29Z

@pomegranited edX is actively still working on more things in this space, right now we're forking Celery to dig into what's going on here and to add better logging. The current theory is that import failures during Celery's autodiscovery are being swallowed by celery. We'll hopefully know more in the next week or two.

pomegranited · 2020-12-01T05:41:37Z

Thanks for the information @feanil !

I've started watching for PRs against https://github.com/edx/celery, plus https://github.com/edx/edx-platform/pull/25699 and https://github.com/edx/edx-platform/pull/25698

bradenmacdonald · 2021-06-10T17:35:51Z

lms/envs/common.py

-# Celery's task autodiscovery won't find tasks nested in a tasks package.
-# Tasks are only registered when the module they are defined in is imported.
-CELERY_IMPORTS = (
-    'poll.tasks',


Hey @feanil , edX has gotten reports that XBlock-poll's reporting features are no longer working (TNL-8370), and I think it was caused by this change (but went unnoticed for months?).

The issue is that the poll XBlock is not a django app, so its tasks.py won't get auto-discovered by celery. If so, is it OK if I fix it with a PR here to reinstate CELERY_IMPORTS = ('poll.tasks', ), or is there some other way I should be doing that now?

Previous PR: https://github.com/edx/edx-platform/pull/23700

Hey Braden, that seems like a fine fix, the other option would be to make the polls code into a django app? Which I would prefer but either is acceptable.

@alfredchavez

…feature is not working [TNL-8370] [MNG-2273] [BB-4877] ## Description This is a backport of https://github.com/edx/edx-platform/pull/28019 to the Lilac master branch, as [requested](https://github.com/edx/edx-platform/pull/28019#issuecomment-952413371) by @alfredchavez. Summary: > xblock-poll's celery task was broken, then fixed in #23700, then broken again by #25479. This fixes it again. ## Supporting information See original PR. ## Testing instructions See original PR. ## Deadline None ## Other information .

feanil requested review from cpennington and jmbowman October 29, 2020 14:22

ormsbee reviewed Oct 29, 2020

View reviewed changes

cms/envs/common.py Outdated Show resolved Hide resolved

jmbowman approved these changes Oct 29, 2020

View reviewed changes

cms/envs/common.py Outdated Show resolved Hide resolved

feanil force-pushed the feanil/update_celery_config branch from e77cfb5 to c21c172 Compare October 29, 2020 14:26

cpennington reviewed Oct 29, 2020

View reviewed changes

feanil merged commit 2175d00 into master Oct 29, 2020

feanil deleted the feanil/update_celery_config branch October 29, 2020 14:52

ormsbee mentioned this pull request Oct 30, 2020

Load celery-specific AppConfigs for bulk_email, instructor_task. #25478

Closed

pomegranited mentioned this pull request Nov 28, 2020

SE-2010 Allows XBlocks to order columns for the Problem Response Report #23109

Merged

2 tasks

mtyaka mentioned this pull request Dec 7, 2020

Update Celery configuration to keep using RabbitMQ as the backend open-craft/opencraft#676

Merged

1 task

pomegranited mentioned this pull request Dec 9, 2020

SE-2010 Implements the XBlock problem response report with a detailed assessment and responses report openedx/edx-ora2#1350

Merged

2 tasks

bradenmacdonald reviewed Jun 10, 2021

View reviewed changes

bradenmacdonald mentioned this pull request Jun 24, 2021

fix: xblock-poll's export to CSV feature is not working [TNL-8370] [MNG-2273] #28019

Merged

bradenmacdonald mentioned this pull request Oct 29, 2021

Backport to Lilac master: xblock-poll's export to CSV feature is not working [TNL-8370] [MNG-2273] [BB-4877] #29173

Merged

Update celery task discovery config. #25479

Update celery task discovery config. #25479

Uh oh!

Conversation

feanil commented Oct 29, 2020

Uh oh!

Uh oh!

jmbowman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

feanil commented Oct 29, 2020

Uh oh!

cpennington Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

edx-status-bot commented Oct 29, 2020

Uh oh!

edx-pipeline-bot commented Oct 29, 2020

Uh oh!

edx-pipeline-bot commented Oct 29, 2020

Uh oh!

edx-pipeline-bot commented Oct 29, 2020

Uh oh!

edx-pipeline-bot commented Oct 29, 2020

Uh oh!

edx-pipeline-bot commented Oct 29, 2020

Uh oh!

pomegranited commented Nov 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feanil commented Nov 30, 2020

Uh oh!

pomegranited commented Dec 1, 2020

Uh oh!

bradenmacdonald Jun 10, 2021

Choose a reason for hiding this comment

Uh oh!

feanil Jun 24, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

pomegranited commented Nov 28, 2020 •

edited

Loading