-
Notifications
You must be signed in to change notification settings - Fork 429
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
feat(tasks-processor): Add recurring task to clean up old recurring task runs #3151
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
register_recurring_task, | ||
register_task_handler, | ||
) | ||
from task_processor.models import HealthCheckModel, Task | ||
from task_processor.models import HealthCheckModel, RecurringTaskRun, Task | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -50,3 +50,17 @@ def clean_up_old_tasks(): | |
0 : settings.TASK_DELETE_BATCH_SIZE # noqa:E203 | ||
] | ||
).delete() | ||
|
||
|
||
@register_recurring_task( | ||
run_every=settings.TASK_DELETE_RUN_EVERY, | ||
first_run_time=settings.TASK_DELETE_RUN_TIME, | ||
) | ||
def clean_up_old_recurring_task_runs(): | ||
if not settings.ENABLE_CLEAN_UP_OLD_TASKS: | ||
return | ||
|
||
now = timezone.now() | ||
delete_before = now - timedelta(days=settings.RECURRING_TASK_RUN_RETENTION_DAYS) | ||
|
||
RecurringTaskRun.objects.filter(finished_at__lt=delete_before).delete() | ||
Comment on lines
+55
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason that we shouldn't just use the same task as the one which removes regular tasks / task runs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's because of the name of the task mostly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to use the same retention period for both types of task?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we probably want to control this independently, e.g: we can probably go with a lower value here because it's not as critical as tasks?