feat: disable PeriodicTasks removed from beat_schedule#1035
Open
serl wants to merge 2 commits intocelery:mainfrom
Open
feat: disable PeriodicTasks removed from beat_schedule#1035serl wants to merge 2 commits intocelery:mainfrom
serl wants to merge 2 commits intocelery:mainfrom
Conversation
Track tasks imported from CELERY_BEAT_SCHEDULE with a new from_configuration boolean on PeriodicTask. On beat startup, rows flagged from_configuration=True whose name is no longer in the config are disabled (not deleted) so history is preserved and admin/ORM created tasks are never touched. Surface the flag in the admin and warn editors that changes to imported tasks are reverted on the next beat restart. Closes celery#248, celery#654. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Make CELERY_BEAT_SCHEDULE the source of truth for the enabled flag on imported tasks: removing-then-re-adding a task to the configuration re-enables the existing row, instead of leaving it stuck in the auto-disabled state from the previous orphan-cleanup pass. The admin warning banner already advertises this revert-on-restart behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds explicit tracking for periodic tasks imported from CELERY_BEAT_SCHEDULE and changes scheduler startup behavior to disable (not delete) DB rows that are no longer present in configuration, preserving history and avoiding impacting admin/ORM-created tasks.
Changes:
- Add
PeriodicTask.from_configurationboolean field (with migration) and mark imported tasks accordingly. - On beat startup, disable
from_configuration=Truetasks missing from current configuration and re-enable when re-added. - Expose the flag in Django admin and show a warning on the change form; expand unit tests and docs to reflect the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
django_celery_beat/schedulers.py |
Marks config-imported rows, forces enabled=True for config entries, and disables orphaned imported tasks on startup. |
django_celery_beat/models.py |
Introduces from_configuration field on PeriodicTask. |
django_celery_beat/migrations/0020_periodictask_from_configuration.py |
Adds the DB migration for from_configuration. |
django_celery_beat/admin.py |
Shows from_configuration in admin and warns when editing imported tasks. |
t/unit/test_schedulers.py |
Updates/extends tests for re-enable behavior, orphan disabling, and admin warning message. |
docs/includes/introduction.txt |
Documents how imported tasks behave on restart and how admin edits are handled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| installed_names = set() | ||
| installed_names |= self.install_default_entries(self.schedule) | ||
| installed_names |= self.update_from_dict(self.app.conf.beat_schedule) | ||
| self._disable_removed_from_configuration(installed_names) |
Comment on lines
483
to
489
| for name, entry_fields in mapping.items(): | ||
| try: | ||
| entry = self.Entry.from_entry(name, | ||
| app=self.app, | ||
| **entry_fields) | ||
| installed.add(name) | ||
| if entry.model.enabled: |
Comment on lines
+650
to
+656
| self.Scheduler(app=self.app) | ||
|
|
||
| # Row preserved, but disabled. last_run_at cleared. | ||
| task = PeriodicTask.objects.get(name=self.entry_name) | ||
| assert task.enabled is False | ||
| assert task.from_configuration is True | ||
| assert task.last_run_at is None |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Track tasks imported from CELERY_BEAT_SCHEDULE with a new from_configuration boolean on PeriodicTask.
On beat startup, rows flagged from_configuration=True whose name is no longer in the config are disabled (not deleted) so history is preserved and admin/ORM created tasks are never touched.
Those tasks are re-enabled when re-added to the configuration.
NOTE: This changes the behavior, as the
enabledflag was not overridden on worker restart before. Specifically, if a task is inCELERY_BEAT_SCHEDULEand manually disabled in the admin, this would re-enable it. A way out would be to hard delete the deleted tasks (and stop manipulatingenabled)On the other hand, this PR also shows the flag in the admin and warns editors that changes to imported tasks are reverted on the next beat restart.
Closes #248, #654.
Linked to #1034, IMO both are interesting to have, but no hard feelings either way (or no way 😁)