Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to celery beat through custom Schedulers #65

Merged
merged 4 commits into from
Mar 30, 2023
Merged

Add support to celery beat through custom Schedulers #65

merged 4 commits into from
Mar 30, 2023

Conversation

edg956
Copy link
Contributor

@edg956 edg956 commented Mar 24, 2023

As suggested in the documentation and in #9, one can set a dispatching task as a proxy to run tasks within all tenant contexts in order to work with beat and periodic tasks. This is a simple solution which might be applicable in smaller or new projects, but might be something you want to avoid in bigger projects.

We did not want to rewrite all the scheduled tasks to depend on a dispatching task, so we looked into how we could change the behaviour of beat without touching the configuration or the tasks and came up with the idea of subclassing Scheduler to control just that.

This PR adds a TenantAwareSchedulerMixin that is used to create TenantAwareScheduler and TenantAwarePersistentScheduler (celery's built-in schedulers), which are responsible for dispatching the task to all tenants. Now, switching to a tenant-aware dispatching mechanism only involves selecting the specific scheduler in the invocation to celery beat with the --scheduler flag (see README.md).

In addition, we also wanted to enable some tasks only for some tenants - though our use case is a bit different -, which comes in handy when you want to run a task on the public schema. For that matter, we also added a TenantAwareSchedulerEntry class that allows users to set a new config tenant_schemas in their app.conf.beat_schedule or similar.

.gitignore Outdated Show resolved Hide resolved
@maciej-gol
Copy link
Owner

Thanks a lot for this contribution, looks great!

@maciej-gol
Copy link
Owner

@edg956 could you please rebase with the newest master? Looks like some old packages break the checks,

@edg956
Copy link
Contributor Author

edg956 commented Mar 25, 2023

Didn't have access to a proper environment so I just used github to sync my repo with yours, which does a merge instead of a rebase. Let's see if that's enough 🤞

@edg956
Copy link
Contributor Author

edg956 commented Mar 25, 2023

I ran the tests on master and they passed (except for two, marked as xfail). I'll try to see how my changes break the tests.

edg956 and others added 4 commits March 25, 2023 21:22
The subclasses of this mixin (provided they're mixed up with `PersistentScheduler` or `Scheduler`) call the task's `delay` function inside of each tenant's context
@edg956
Copy link
Contributor Author

edg956 commented Mar 25, 2023

Ok, I got them to pass. I had just assumed it was a problem with my local environment, apologies for that!

The fix was as simple as adding set_as_current=False to the initialization of CeleryApp. Since I wasn't configuring my app with the settings from DJANGO_SETTINGS_MODULE and celery by default sets any new instance as the current app, the app the integration tests were using wasn't configured to point to rabbit, etc.

Also, I reorganised the commits so it looks like I rebased from master instead of merging

@maciej-gol maciej-gol merged commit a2bbd18 into maciej-gol:master Mar 30, 2023
@maciej-gol
Copy link
Owner

Thanks a lot for this contribution! Released in 2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants