feat!: don't run Celery workers in dev mode
Tutor's importing * from devstack.py[1] for the development settings, and that means that we aren't using Celery workers at all in dev mode (see [2]). This removes them from the dev compose file, thus saving everyboding a significant chunk of RAM.
[1] https://github.com/overhangio/tutor/blob/master/tutor/templates/apps/openedx/settings/lms/development.py#L3 [2] https://github.com/openedx/edx-platform/blob/master/lms/envs/devstack.py#L35
To do so, we rely on Docker compose profiles[3].
[3] https://docs.docker.com/compose/profiles/
BREAKING CHANGE: the COMPOSE_PROJECT_STARTED hook signature had to be changed to accomodate profile selection.
Haha, I have a competing proposal (although I let it languish, because I was never able to confirm or deny that pdb would work on these workers):
- https://github.com/overhangio/tutor/pull/928
My thinking was that it would be good to run these tasks asynchronously, because it would expose developers to the same race conditions that inevitably always happen with Celery tasks in production. But I can see the other side of the argument: async tasks are harder to debug and the workers use resources.
Either way, I think we can all agree that we should either use these workers, or turn them off. So I see four paths forward:
- Use this PR, which will disable workers in dev. Open edX contributors who work on Celery tasks should take note to manually test their code using
tutor local. - Use my PR, which will enable Celery tasks by default for everybody.
- Create a new Tutor
ASYNC_CELERY_TASKSconfiguration flag, enabled by default. Disabling it would remove the Celery workers from docker-compose and configure Django to run the tasks in-process. - Same as (3), but implement it as a
tutor-celeryplugin. Like tutor-mfe, it would be installed and enabled by default.
My vote is (1), but I would support any of them.
Haha, I have a competing proposal (although I let it languish, because I was never able to confirm or deny that
pdbwould work on these workers):My thinking was that it would be good to run these tasks asynchronously, because it would expose developers to the same race conditions that inevitably always happen with Celery tasks in production. But I can see the other side of the argument: async tasks are harder to debug and the workers use resources.
Either way, I think we can all agree that we should either use these workers, or turn them off. So I see four paths forward:
- Use this PR, which will disable workers in dev. Open edX contributors who work on Celery tasks should take note to manually test their code using
tutor local.- Use my PR, which will enable Celery tasks by default for everybody.
- Create a new Tutor
ASYNC_CELERY_TASKSconfiguration flag, enabled by default. Disabling it would remove the Celery workers from docker-compose and configure Django to run the tasks in-process.- Same as (3), but implement it as a
tutor-celeryplugin. Like tutor-mfe, it would be installed and enabled by default.My vote is (1), but I would support any of them. My take on all points:
- This seems like the most viable solution. However, I also 2nd the fact that devs should be able to run the tasks async if they want to. If they can do so using
local, good enough. But it will confuse devs as we suggestdevfor primarily development purposes. Switching to local results in images being rebuilt and all, which can be infuriating if you are trying to debug. - Already discussed in 1
- Régis has argued to avoid adding new configs and instead add filters/plugins if needed.
- How do you envision the plugin would work/would be responsible for within the ecosystem? To relate with 1, if we can use the plugin for this purpose, it would be nice. I suspect we would be using the plugin to override IDA's celery settings and also allow celery configs to be added without modification to the core (sort-of similar situation Régis is describing on https://github.com/overhangio/tutor/pull/1010#discussion_r1568736090)
@arbrandes Hi, what's the plan for this PR? Thanks
@DawoudSheraz, good question. I still used it locally all the time. I figure I could implement the alternative I describe above that has a smaller footprint (setting a fake docker profile for the workers in the dev override docker-compose), which addresses Regis' concerns and gets us the same immediate effect of not having workers in dev. (But we lose the option of using profiles for other things in the future.)
I'll issue a separate PR so we can compare.
Hey Adolfo, your PR completely fell off my radar, I'm sorry about that. A recent issue (#1126) has drawn my attention on celery again. I found a not-so-hackish way of disabling lms-worker and cms-worker in development, without using profiles. You can check out my PR here: https://github.com/overhangio/tutor/pull/1128
While I do agree that profiles are super interesting, and we should probably use them in the future instead of multiple docker-compose files, I think it's a much bigger change that needs extensive testing. Getting rid of celery workers in dev is an obvious improvement for everyone, and I don't want to wait longer to merge it.
Closing in favour of #1128.