Skip to content

fix: get CELERY_RESULT_BACKEND from ENV_TOKENS#30244

Merged
giovannicimolin merged 1 commit intoopenedx:masterfrom
open-craft:navin/bb-6077
Jun 15, 2022
Merged

fix: get CELERY_RESULT_BACKEND from ENV_TOKENS#30244
giovannicimolin merged 1 commit intoopenedx:masterfrom
open-craft:navin/bb-6077

Conversation

@navinkarkera
Copy link
Contributor

@navinkarkera navinkarkera commented Apr 14, 2022

Description

At Opencraft, we want to run multiple instances of celery workers in separate servers. The celery workers need to be able to share task results. Currently, the CELERY_RESULT_BACKEND is hard coded to use django-cache.

So this PR allows to setup CELERY_RESULT_BACKEND via configuration.

To use django ORM as celery backend, one can set CELERY_RESULT_BACKEND to django-db as described in docs.

Supporting information

Deadline

"None"

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Apr 14, 2022
@openedx-webhooks
Copy link

openedx-webhooks commented Apr 14, 2022

Thanks for the pull request, @navinkarkera! I've created OSPR-6618 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

This was referenced Apr 14, 2022
Merged
Merged
Merged
Merged
@natabene
Copy link
Contributor

natabene commented Apr 26, 2022

@navinkarkera Thank you for your contribution. Is this ready for our review?

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Apr 26, 2022
@kaustavb12
Copy link
Contributor

kaustavb12 commented Apr 27, 2022

Hi @natabene !

This is not ready for your review yet. I'll ping you here once ready.

kaustavb12
kaustavb12 approved these changes May 22, 2022
View reviewed changes
Copy link
Contributor

@kaustavb12 kaustavb12 left a comment

Choose a reason for hiding this comment

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

👍

Looks good to me

  • I tested this: Tested in the PR sandbox
  • I read through the code

@kaustavb12
Copy link
Contributor

kaustavb12 commented May 22, 2022 •
edited
Loading

@natabene

Thank you for your patience here. This PR is now ready for your review.

@navinkarkera navinkarkera mentioned this pull request Jun 8, 2022
Merged
giovannicimolin
giovannicimolin reviewed Jun 9, 2022
View reviewed changes
giovannicimolin
giovannicimolin approved these changes Jun 10, 2022
View reviewed changes
Copy link
Contributor

@giovannicimolin giovannicimolin left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this:
  1. Checked that the default value retains previous behavior.
  2. Checked that changing the variable allows configuring a different result backend for celery (verified it's working on a client instance)
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation: comment on code.

@navinkarkera Looks good 👍
I'll wait two days before merging this to see if anyone raises concerns.

CC @natabene

This was referenced Jun 13, 2022
Merged
Merged
@giovannicimolin giovannicimolin merged commit b50f3ec into openedx:master Jun 15, 2022
@openedx-webhooks
Copy link

openedx-webhooks commented Jun 15, 2022

@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@openedx-webhooks openedx-webhooks removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jun 15, 2022
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jun 15, 2022

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

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jun 15, 2022

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 #warroom 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
Copy link
Contributor

edx-pipeline-bot commented Jun 15, 2022

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

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jun 15, 2022

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

github-actions bot added a commit that referenced this pull request Aug 2, 2022
@github-actions
merge(#30572): refactor: [BB-6077] allow setting celery backend in yml
294c1f3 
<!--

🌰🌰
🌰🌰🌰🌰         🌰 Note: the Nutmeg master branch has been created.  Please consider whether your change
    🌰🌰🌰🌰     should also be applied to Nutmeg. If so, make another pull request against the
🌰🌰🌰🌰         open-release/nutmeg.master branch, or ping @nedbat for help or questions.
🌰🌰

Please give your pull request a short but descriptive title.
Use conventional commits to separate and summarize commits logically:
https://open-edx-proposals.readthedocs.io/en/latest/oep-0051-bp-conventional-commits.html

Use this template as a guide. Omit sections that don't apply. You may link to information rather than copy it.
More details about the template are at openedx/openedx-proposals#180
(link will be updated when that document merges)
-->

## Description
**Cherry pick of #30244

At Opencraft, we want to run multiple instances of celery workers in separate servers. The celery workers need to be able to share task results. Currently, the `CELERY_RESULT_BACKEND` is hard coded to use `django-cache`.

So this PR allows to setup `CELERY_RESULT_BACKEND` via configuration.

To use django ORM as celery backend, one can set `CELERY_RESULT_BACKEND` to `django-db` as described in [docs](https://docs.celeryq.dev/en/stable/django/first-steps-with-django.html#django-celery-results-using-the-django-orm-cache-as-a-result-backend).

## Supporting information

- Task: https://tasks.opencraft.com/browse/BB-6077

## Deadline

"None"
@0x29a 0x29a mentioned this pull request Sep 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@giovannicimolin giovannicimolin giovannicimolin approved these changes

@kaustavb12 kaustavb12 kaustavb12 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@giovannicimolin giovannicimolin

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Contributions
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@navinkarkera @openedx-webhooks @natabene @kaustavb12 @edx-pipeline-bot @giovannicimolin