[SE-4860] make SINGLE_BEAT_IDENTIFIER configurable by gabor-boros · Pull Request #6586 · openedx-unsupported/configuration

gabor-boros · 2021-10-18T05:43:45Z

Description

This PR allows setting separate users for single-beat and celery brokers, allows authentication, and setting the single-beat identifier for future fine-grained access control using Redis ACLs.

Also, we are replacing the source url of the package, hence the original code became outdated and akachanov:master is ahead of the original repo by fixing issues and removing incompatible code.

Supporting information

Some months ago edX platform was not capable of doing periodic scheduling of celery tasks. Therefore we added celerybeat to the platform. The only issue with celerybeat is that it cannot run more than once. If multiple celerybeat schedulers are running at the same time, the same task can be scheduled multiple times resulting in unwanted behaviours. Here comes single-beat into picture that ensures (using redis) that no more than one celerybeat processes are running.

Can you explain what it is in akachanov's fork that's critical to us here?

Akachanov removed Tornadis that was published in 2018 for the last time and broke with the newest Celery. Using Akachanov's fork ensures that single-beat is still working with the newest Celery and he also seems to fixed some reconnect issues -- I assume hence the celerybeat process restarts, since I cannot see any errors and periodically scheduled tasks are processed as expected.

Dependencies

Related to https://github.com/edx/edx-platform/pull/29046, which upgrades celery in edx-platform.

Sandbox

https://pr29046.sandbox.opencraft.hosting

Testing instructions

Navigate to https://pr29046.sandbox.opencraft.hosting/heartbeat?extended and validate it is working properly
Go to https://pr29046.sandbox.opencraft.hosting/admin/instructor_task/instructortask/ and validate reports are generated every 10 minutes

Deadline

ASAP

Other information

N/A

openedx-webhooks · 2021-10-18T05:43:52Z

Thanks for the pull request, @gabor-boros! I've created OSPR-6145 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.

natabene · 2021-10-18T15:05:19Z

@gabor-boros Thank you for your contribution. Please let me know once this is ready for our review.

pomegranited

@gabor-boros This change seems to do more than just make SINGLE_BEAT_IDENTIFIER configurable.. Have asked for clarity, and suggested some changes.

pomegranited

👍

I tested this on the PR sandbox.
I read through the code
~~I checked for accessibility issues~~ N/A
~~Includes documentation~~ N/A -- as per the PR template checklist, this PR does not add any defaults which need to be overridden, so no CHANGELOG entry is necessary.
I made sure any change in configuration variables is reflected in the corresponding
client's configuration-secure repository. N/A

@jmbowman Can you confirm that edX is not using celerybeat and so this change should not affect you guys?

I'll merge this as core commiter in 2 days if there are no objections on your side. CC @natabene

Agrendalath

👍

I tested this: I've checked heartbeats and periodic tasks in the sandbox
I read through the code
I checked for accessibility issues: n/a
Includes documentation: no ⚠️
I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

gabor-boros · 2021-11-30T14:24:19Z

@Agrendalath I believe all the requested changes are addressed. The app server is provisioning with the new changes.

Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

openedx-webhooks · 2021-12-07T00:31:24Z

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

…ed#6586) * feat: make SINGLE_BEAT_IDENTIFIER configurable * fix: celery 5.0+ does not support --app as a worker flag * refactor: remove duplicated app declaration * fix: --config flag is global since Celery 5.0+ * fix: replace single-beat with an updated version * fix: add missing comma * fix: replace maxtasksperchild worker flag * refactor: address PR review comments Signed-off-by: Gabor Boros <gabor.brs@gmail.com> * refactor: remove celery<5 adjustments Signed-off-by: Gabor Boros <gabor.brs@gmail.com> * fix: set single-beat autorestart Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

…upported#6586) * feat: make SINGLE_BEAT_IDENTIFIER configurable * fix: celery 5.0+ does not support --app as a worker flag * refactor: remove duplicated app declaration * fix: --config flag is global since Celery 5.0+ * fix: replace single-beat with an updated version * fix: add missing comma * fix: replace maxtasksperchild worker flag * refactor: address PR review comments Signed-off-by: Gabor Boros <gabor.brs@gmail.com> * refactor: remove celery<5 adjustments Signed-off-by: Gabor Boros <gabor.brs@gmail.com> * fix: set single-beat autorestart Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Oct 18, 2021

This was referenced Oct 18, 2021

[SE-4860] Upgrade Celery to v5.2.1 openedx/openedx-platform#29046

Merged

feat: configure single-beat for Redis users open-craft/opencraft#847

Merged

openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Oct 18, 2021

gabor-boros mentioned this pull request Oct 20, 2021

[SE-4860] fix celery upgrade import mckinseyacademy/xblock-poll#22

Closed

gabor-boros force-pushed the gabor/add-single-beat-auth branch from af33d26 to b62c528 Compare October 21, 2021 14:24

gabor-boros mentioned this pull request Oct 21, 2021

feat: upgrade poll XBlock to use celery 5.0+ features open-craft/xblock-poll#95

Closed

gabor-boros mentioned this pull request Nov 9, 2021

Ensure continuous support of Django 3.2 and celery v5.0+ open-craft/xblock-poll#96

Merged

pomegranited suggested changes Nov 23, 2021

View reviewed changes

pomegranited approved these changes Nov 24, 2021

View reviewed changes

pomegranited added cc-reviewer and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Nov 24, 2021

Agrendalath approved these changes Nov 26, 2021

View reviewed changes

Comment thread playbooks/roles/ecomworker/templates/edx/app/ecomworker/ecomworker.sh.j2 Outdated

Comment thread ...s/roles/edx_django_service/templates/edx/app/supervisor/conf.d.available/app-workers.conf.j2 Outdated

Comment thread playbooks/roles/edxapp/defaults/main.yml

openedx-webhooks added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Nov 26, 2021

gabor-boros added 8 commits November 30, 2021 15:14

feat: make SINGLE_BEAT_IDENTIFIER configurable

f501127

fix: celery 5.0+ does not support --app as a worker flag

780e2bf

refactor: remove duplicated app declaration

c1f42ce

fix: --config flag is global since Celery 5.0+

f95aa0b

fix: replace single-beat with an updated version

b0eb6b7

fix: add missing comma

6979245

fix: replace maxtasksperchild worker flag

375453a

refactor: address PR review comments

f7a5fc9

Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

gabor-boros force-pushed the gabor/add-single-beat-auth branch from b62c528 to f7a5fc9 Compare November 30, 2021 14:18

refactor: remove celery<5 adjustments

bc640a2

Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

fix: set single-beat autorestart

a8fd3f8

Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

gabor-boros force-pushed the gabor/add-single-beat-auth branch from e71f18b to a8fd3f8 Compare December 1, 2021 10:53

pomegranited merged commit b8ab26c into openedx-unsupported:master Dec 7, 2021

openedx-webhooks added merged and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Dec 7, 2021

pomegranited deleted the gabor/add-single-beat-auth branch December 7, 2021 00:32

pomegranited self-assigned this Dec 8, 2021

gabor-boros restored the gabor/add-single-beat-auth branch March 13, 2022 20:48

This was referenced Mar 13, 2022

[SE-4860] cherry-pick configuration/pull/6586 open-craft/configuration#185

Closed

[SE-4860] cherry-pick configuration/pull/6586 open-craft/configuration#186

Merged

0x29a mentioned this pull request May 4, 2022

[BB-6112] Prepare opencraft-release/maple.3 branches for configuration open-craft/configuration#193

Merged

Agrendalath deleted the gabor/add-single-beat-auth branch May 13, 2022 11:24

navinkarkera mentioned this pull request Jun 8, 2022

[BB-6123] cherry-pick of celery upgrade configurations open-craft/configuration#198

Merged

Cup0fCoffee mentioned this pull request Sep 28, 2022

Prepare opencraft-release/nutmeg.2 branch for configuration open-craft/configuration#207

Merged

Navigation Menu

Uh oh!

[SE-4860] make SINGLE_BEAT_IDENTIFIER configurable - #6586

[SE-4860] make SINGLE_BEAT_IDENTIFIER configurable#6586
pomegranited merged 10 commits into
openedx-unsupported:masterfrom
open-craft:gabor/add-single-beat-auth

gabor-boros commented Oct 18, 2021 •

edited by pomegranited

Loading

Uh oh!

openedx-webhooks commented Oct 18, 2021

Uh oh!

natabene commented Oct 18, 2021

Uh oh!

pomegranited left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pomegranited left a comment

Uh oh!

Agrendalath left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gabor-boros commented Nov 30, 2021

Uh oh!

openedx-webhooks commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

gabor-boros commented Oct 18, 2021 • edited by pomegranited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Dependencies

Sandbox

Testing instructions

Deadline

Other information

Uh oh!

openedx-webhooks commented Oct 18, 2021

Uh oh!

natabene commented Oct 18, 2021

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

Agrendalath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gabor-boros commented Nov 30, 2021

Uh oh!

openedx-webhooks commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gabor-boros commented Oct 18, 2021 •

edited by pomegranited

Loading