appsembler / configuration

a simple, but flexible, way for anyone to stand up an instance of the edX platform that is fully configured and ready-to-go
GNU Affero General Public License v3.0
14 stars 13 forks source link

Fix CMS celerybeat Supervisor config #377

Closed bryanlandia closed 3 years ago

bryanlandia commented 3 years ago

It was using pidfile and schedule from LMS Celerybeat config Not worrying about upstream since it has changed to use single-beat

Configuration Pull Request

Make sure that the following steps are done before merging:

OmarIthawi commented 3 years ago

Thanks @bryanlandia. We use Celerybeat for Figures on Tahoe SaaS, so I'll have to ask @johnbaldwin for reviewing this change.

The other best reviewers are either Maxi or Anders.

Meanwhile, I would love to know why the change has been done and a little bit of explanation to understand what the change does. I'm asking this because the change looks completely okay, but I don't understand what it does.

bryanlandia commented 3 years ago

Thanks @OmarIthawi . Turns out configuration repo really only sets up LMS Celerybeat correctly. For InterSys we have a CMS only Django app and I want CMS Celerybeat and workers to handle its scheduled tasks

bryanlandia commented 3 years ago

Thanks for the review @thraxil John, please take a look as well with Figures in mind

johnbaldwin commented 3 years ago

I don't see any issues here with regard to Figures.

Figures should be agnostic to specifics for celerybeat and uses the Django settings. Code here: https://github.com/appsembler/figures/blob/master/figures/settings/lms_production.py#L123-L138