From 1aa90adac3cb1764fed2de15465e1215a9c956be Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 14:51:33 +1000 Subject: [PATCH 01/10] Cron Offset Module --- allianceauth/framework/cron.py | 47 +++++++++++++++++++ .../framework/migrations/0001_initial.py | 28 +++++++++++ allianceauth/framework/migrations/__init__.py | 0 allianceauth/framework/models.py | 19 ++++++++ 4 files changed, 94 insertions(+) create mode 100644 allianceauth/framework/cron.py create mode 100644 allianceauth/framework/migrations/0001_initial.py create mode 100644 allianceauth/framework/migrations/__init__.py create mode 100644 allianceauth/framework/models.py diff --git a/allianceauth/framework/cron.py b/allianceauth/framework/cron.py new file mode 100644 index 00000000..044247cf --- /dev/null +++ b/allianceauth/framework/cron.py @@ -0,0 +1,47 @@ +from celery.schedules import crontab +import logging +from allianceauth.framework.models import CronOffset +from django.db import ProgrammingError + +logger = logging.getLogger(__name__) + + +# CELERYBEAT_SCHEDULE['marketmanager_fetch_public_market_orders'] = { +# 'task': 'marketmanager.tasks.fetch_public_market_orders', +# 'schedule': crontab(minute=0, hour='*/3'), +# } + + +def offset_cron(schedule: crontab) -> crontab: + """Take a crontab and apply a series of precalculated offsets to spread out tasks execution on remote resources + + Args: + schedule (crontab): celery.schedules.crontab() + + Returns: + crontab: A crontab with offsetted Minute and Hour fields + """ + + try: + cron_offset = CronOffset.get_solo() + new_minute = [(m + (round(60 * cron_offset.minute))) % 60 for m in schedule.minute] + new_hour = [(m + (round(24 * cron_offset.hour))) % 24 for m in schedule.hour] + + # Type hints are fine here, crontab _expand_cronspec handles sets, the hinting is old + return crontab( + minute=",".join(str(m) for m in sorted(new_minute)), + hour=",".join(str(h) for h in sorted(new_hour)), + day_of_month=schedule._orig_day_of_month, + month_of_year=schedule._orig_month_of_year, + day_of_week=schedule._orig_day_of_week) + + except ProgrammingError as e: + # If this is called before migrations are run hand back the default schedule + # These offsets are stored in a Singleton Model, + logger.error(e) + return schedule + + except Exception as e: + # We absolutely cant fail to hand back a schedule + logger.error(e) + return schedule diff --git a/allianceauth/framework/migrations/0001_initial.py b/allianceauth/framework/migrations/0001_initial.py new file mode 100644 index 00000000..e3fb6d2d --- /dev/null +++ b/allianceauth/framework/migrations/0001_initial.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.16 on 2024-12-29 04:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='CronOffset', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('minute', models.FloatField(default=0.1850723825351236, verbose_name='Minute Offset')), + ('hour', models.FloatField(default=0.3449986529941984, verbose_name='Hour Offset')), + ('day_of_month', models.FloatField(default=0.8941823028373547, verbose_name='Day of Month Offset')), + ('month_of_year', models.FloatField(default=0.6068269517452, verbose_name='Month Of Year Offset')), + ('day_of_week', models.FloatField(default=0.8863389239634608, verbose_name='Day of Week Offset')), + ], + options={ + 'verbose_name': 'Cron Offsets', + }, + ), + ] diff --git a/allianceauth/framework/migrations/__init__.py b/allianceauth/framework/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/allianceauth/framework/models.py b/allianceauth/framework/models.py new file mode 100644 index 00000000..287a13de --- /dev/null +++ b/allianceauth/framework/models.py @@ -0,0 +1,19 @@ +from random import random +from django.db import models +from django.utils.translation import gettext_lazy as _ +from solo.models import SingletonModel + + +class CronOffset(SingletonModel): + + minute = models.FloatField(_("Minute Offset"), default=random()) + hour = models.FloatField(_("Hour Offset"), default=random()) + day_of_month = models.FloatField(_("Day of Month Offset"), default=random()) + month_of_year = models.FloatField(_("Month Of Year Offset"), default=random()) + day_of_week = models.FloatField(_("Day of Week Offset"), default=random()) + + def __str__(self) -> str: + return "Cron Offsets" + + class Meta: + verbose_name = "Cron Offsets" From 8fd1411f09978a708fb3e19d6893ea27a04a2238 Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 14:51:57 +1000 Subject: [PATCH 02/10] shift schedule to ready() in order to access DB --- allianceauth/apps.py | 47 ++++++++++++++++++- .../project_name/settings/base.py | 23 +-------- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/allianceauth/apps.py b/allianceauth/apps.py index 098f50ba..3a831f59 100644 --- a/allianceauth/apps.py +++ b/allianceauth/apps.py @@ -1,5 +1,5 @@ from django.apps import AppConfig -from django.core.checks import Warning, Error, register +from celery.schedules import crontab class AllianceAuthConfig(AppConfig): @@ -7,3 +7,48 @@ class AllianceAuthConfig(AppConfig): def ready(self) -> None: import allianceauth.checks # noqa + from django_celery_beat.models import CrontabSchedule, PeriodicTask + from allianceauth.framework.cron import offset_cron + + PeriodicTask.objects.update_or_create( + name='esi_cleanup_callbackredirect', + defaults={ + 'task': 'esi.tasks.cleanup_callbackredirect', + 'crontab': CrontabSchedule.objects.get_or_create(minute='0', hour='0', day_of_week='*', day_of_month='*', month_of_year='*', timezone='UTC')[0], + }, + ) + PeriodicTask.objects.update_or_create( + name='esi_cleanup_token', + defaults={ + 'task': 'esi.tasks.cleanup_token', + 'crontab': CrontabSchedule.objects.get_or_create(minute='0', hour='0', day_of_week='*', day_of_month='*', month_of_year='*', timezone='UTC')[0], + }, + ) + + z = CrontabSchedule.from_schedule(offset_cron(crontab(minute='0', hour='*/6'))) + PeriodicTask.objects.update_or_create( + name='run_model_update', + defaults={ + 'task': 'allianceauth.eveonline.tasks.run_model_update', + 'crontab': CrontabSchedule.objects.get_or_create( # Convert the offsetted cron into a DB object + minute=z.minute, hour=z.hour, day_of_week=z.day_of_week, day_of_month=z.day_of_month, month_of_year=z.month_of_year, timezone=z.timezone)[0], + }, + ) + + z = CrontabSchedule.from_schedule(offset_cron(crontab(minute='0', hour='*/4'))) + PeriodicTask.objects.update_or_create( + name='check_all_character_ownership', + defaults={ + 'task': 'allianceauth.authentication.tasks.check_all_character_ownership', + 'crontab': CrontabSchedule.objects.get_or_create( # Convert the offsetted cron into a DB object + minute=z.minute, hour=z.hour, day_of_week=z.day_of_week, day_of_month=z.day_of_month, month_of_year=z.month_of_year, timezone=z.timezone)[0], + }, + ) + PeriodicTask.objects.update_or_create( + name='analytics_daily_stats', + defaults={ + 'task': 'allianceauth.analytics.tasks.analytics_daily_stats', + 'crontab': CrontabSchedule.objects.get_or_create( + minute='0', hour='12', day_of_week='*', day_of_month='*', month_of_year='*', timezone='UTC')[0], + }, + ) diff --git a/allianceauth/project_template/project_name/settings/base.py b/allianceauth/project_template/project_name/settings/base.py index fb18e4de..77897c99 100644 --- a/allianceauth/project_template/project_name/settings/base.py +++ b/allianceauth/project_template/project_name/settings/base.py @@ -50,28 +50,7 @@ SECRET_KEY = "wow I'm a really bad default secret key" # Celery configuration BROKER_URL = 'redis://localhost:6379/0' CELERYBEAT_SCHEDULER = "django_celery_beat.schedulers.DatabaseScheduler" -CELERYBEAT_SCHEDULE = { - 'esi_cleanup_callbackredirect': { - 'task': 'esi.tasks.cleanup_callbackredirect', - 'schedule': crontab(minute='0', hour='*/4'), - }, - 'esi_cleanup_token': { - 'task': 'esi.tasks.cleanup_token', - 'schedule': crontab(minute='0', hour='0'), - }, - 'run_model_update': { - 'task': 'allianceauth.eveonline.tasks.run_model_update', - 'schedule': crontab(minute='0', hour="*/6"), - }, - 'check_all_character_ownership': { - 'task': 'allianceauth.authentication.tasks.check_all_character_ownership', - 'schedule': crontab(minute='0', hour='*/4'), - }, - 'analytics_daily_stats': { - 'task': 'allianceauth.analytics.tasks.analytics_daily_stats', - 'schedule': crontab(minute='0', hour='2'), - } -} +CELERYBEAT_SCHEDULE = {} # Build paths inside the project like this: os.path.join(BASE_DIR, ...) PROJECT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) From c3fa8acd8ecc2472a33a75633116b20a2b3726bd Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 15:05:53 +1000 Subject: [PATCH 03/10] Tweak migrations to use random function not preset --- allianceauth/framework/migrations/0001_initial.py | 13 +++++++------ allianceauth/framework/models.py | 14 +++++++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/allianceauth/framework/migrations/0001_initial.py b/allianceauth/framework/migrations/0001_initial.py index e3fb6d2d..4888ceda 100644 --- a/allianceauth/framework/migrations/0001_initial.py +++ b/allianceauth/framework/migrations/0001_initial.py @@ -1,5 +1,6 @@ -# Generated by Django 4.2.16 on 2024-12-29 04:19 +# Generated by Django 4.2.16 on 2024-12-29 05:02 +import allianceauth.framework.models from django.db import migrations, models @@ -15,11 +16,11 @@ class Migration(migrations.Migration): name='CronOffset', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('minute', models.FloatField(default=0.1850723825351236, verbose_name='Minute Offset')), - ('hour', models.FloatField(default=0.3449986529941984, verbose_name='Hour Offset')), - ('day_of_month', models.FloatField(default=0.8941823028373547, verbose_name='Day of Month Offset')), - ('month_of_year', models.FloatField(default=0.6068269517452, verbose_name='Month Of Year Offset')), - ('day_of_week', models.FloatField(default=0.8863389239634608, verbose_name='Day of Week Offset')), + ('minute', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Minute Offset')), + ('hour', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Hour Offset')), + ('day_of_month', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Day of Month Offset')), + ('month_of_year', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Month Of Year Offset')), + ('day_of_week', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Day of Week Offset')), ], options={ 'verbose_name': 'Cron Offsets', diff --git a/allianceauth/framework/models.py b/allianceauth/framework/models.py index 287a13de..4210c63d 100644 --- a/allianceauth/framework/models.py +++ b/allianceauth/framework/models.py @@ -4,13 +4,17 @@ from django.utils.translation import gettext_lazy as _ from solo.models import SingletonModel +def random_default() -> float: + return random() + + class CronOffset(SingletonModel): - minute = models.FloatField(_("Minute Offset"), default=random()) - hour = models.FloatField(_("Hour Offset"), default=random()) - day_of_month = models.FloatField(_("Day of Month Offset"), default=random()) - month_of_year = models.FloatField(_("Month Of Year Offset"), default=random()) - day_of_week = models.FloatField(_("Day of Week Offset"), default=random()) + minute = models.FloatField(_("Minute Offset"), default=random_default) + hour = models.FloatField(_("Hour Offset"), default=random_default) + day_of_month = models.FloatField(_("Day of Month Offset"), default=random_default) + month_of_year = models.FloatField(_("Month Of Year Offset"), default=random_default) + day_of_week = models.FloatField(_("Day of Week Offset"), default=random_default) def __str__(self) -> str: return "Cron Offsets" From 30a79362f4d21147f96d678f7c7f97c8ce559a81 Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 15:13:13 +1000 Subject: [PATCH 04/10] remove development note --- allianceauth/framework/cron.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/allianceauth/framework/cron.py b/allianceauth/framework/cron.py index 044247cf..8ec3eb50 100644 --- a/allianceauth/framework/cron.py +++ b/allianceauth/framework/cron.py @@ -5,13 +5,6 @@ from django.db import ProgrammingError logger = logging.getLogger(__name__) - -# CELERYBEAT_SCHEDULE['marketmanager_fetch_public_market_orders'] = { -# 'task': 'marketmanager.tasks.fetch_public_market_orders', -# 'schedule': crontab(minute=0, hour='*/3'), -# } - - def offset_cron(schedule: crontab) -> crontab: """Take a crontab and apply a series of precalculated offsets to spread out tasks execution on remote resources From 7022cb70504f625d0fd9eabcfb5c8362c38c573d Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 15:14:29 +1000 Subject: [PATCH 05/10] not relevant comment --- allianceauth/framework/cron.py | 1 - 1 file changed, 1 deletion(-) diff --git a/allianceauth/framework/cron.py b/allianceauth/framework/cron.py index 8ec3eb50..c8f54cbd 100644 --- a/allianceauth/framework/cron.py +++ b/allianceauth/framework/cron.py @@ -20,7 +20,6 @@ def offset_cron(schedule: crontab) -> crontab: new_minute = [(m + (round(60 * cron_offset.minute))) % 60 for m in schedule.minute] new_hour = [(m + (round(24 * cron_offset.hour))) % 24 for m in schedule.hour] - # Type hints are fine here, crontab _expand_cronspec handles sets, the hinting is old return crontab( minute=",".join(str(m) for m in sorted(new_minute)), hour=",".join(str(h) for h in sorted(new_hour)), From d12d6e7cdb61ab4fbeb70fe041f041712de1127a Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 22:04:52 +1000 Subject: [PATCH 06/10] move to its own module --- allianceauth/apps.py | 2 +- allianceauth/crontab/__init__.py | 3 + allianceauth/crontab/apps.py | 14 ++++ allianceauth/{framework => crontab}/cron.py | 3 +- allianceauth/{framework => crontab}/models.py | 2 +- allianceauth/crontab/tests/__init__.py | 0 allianceauth/crontab/tests/test_cron.py | 79 +++++++++++++++++++ allianceauth/crontab/tests/test_models.py | 66 ++++++++++++++++ .../project_name/settings/base.py | 1 + 9 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 allianceauth/crontab/__init__.py create mode 100644 allianceauth/crontab/apps.py rename allianceauth/{framework => crontab}/cron.py (96%) rename allianceauth/{framework => crontab}/models.py (91%) create mode 100644 allianceauth/crontab/tests/__init__.py create mode 100644 allianceauth/crontab/tests/test_cron.py create mode 100644 allianceauth/crontab/tests/test_models.py diff --git a/allianceauth/apps.py b/allianceauth/apps.py index 3a831f59..c2397c9b 100644 --- a/allianceauth/apps.py +++ b/allianceauth/apps.py @@ -8,7 +8,7 @@ class AllianceAuthConfig(AppConfig): def ready(self) -> None: import allianceauth.checks # noqa from django_celery_beat.models import CrontabSchedule, PeriodicTask - from allianceauth.framework.cron import offset_cron + from allianceauth.crontab.cron import offset_cron PeriodicTask.objects.update_or_create( name='esi_cleanup_callbackredirect', diff --git a/allianceauth/crontab/__init__.py b/allianceauth/crontab/__init__.py new file mode 100644 index 00000000..c621f6d2 --- /dev/null +++ b/allianceauth/crontab/__init__.py @@ -0,0 +1,3 @@ +""" +Alliance Auth Crontab Utilities +""" diff --git a/allianceauth/crontab/apps.py b/allianceauth/crontab/apps.py new file mode 100644 index 00000000..e47e39ed --- /dev/null +++ b/allianceauth/crontab/apps.py @@ -0,0 +1,14 @@ +""" +Crontab App Config +""" + +from django.apps import AppConfig + + +class CrontabConfig(AppConfig): + """ + Crontab App Config + """ + + name = "allianceauth.crontab" + label = "crontab" diff --git a/allianceauth/framework/cron.py b/allianceauth/crontab/cron.py similarity index 96% rename from allianceauth/framework/cron.py rename to allianceauth/crontab/cron.py index c8f54cbd..bbeecddd 100644 --- a/allianceauth/framework/cron.py +++ b/allianceauth/crontab/cron.py @@ -1,10 +1,11 @@ from celery.schedules import crontab import logging -from allianceauth.framework.models import CronOffset +from allianceauth.crontab.models import CronOffset from django.db import ProgrammingError logger = logging.getLogger(__name__) + def offset_cron(schedule: crontab) -> crontab: """Take a crontab and apply a series of precalculated offsets to spread out tasks execution on remote resources diff --git a/allianceauth/framework/models.py b/allianceauth/crontab/models.py similarity index 91% rename from allianceauth/framework/models.py rename to allianceauth/crontab/models.py index 4210c63d..f23b35ea 100644 --- a/allianceauth/framework/models.py +++ b/allianceauth/crontab/models.py @@ -13,7 +13,7 @@ class CronOffset(SingletonModel): minute = models.FloatField(_("Minute Offset"), default=random_default) hour = models.FloatField(_("Hour Offset"), default=random_default) day_of_month = models.FloatField(_("Day of Month Offset"), default=random_default) - month_of_year = models.FloatField(_("Month Of Year Offset"), default=random_default) + month_of_year = models.FloatField(_("Month of Year Offset"), default=random_default) day_of_week = models.FloatField(_("Day of Week Offset"), default=random_default) def __str__(self) -> str: diff --git a/allianceauth/crontab/tests/__init__.py b/allianceauth/crontab/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/allianceauth/crontab/tests/test_cron.py b/allianceauth/crontab/tests/test_cron.py new file mode 100644 index 00000000..c9a2a970 --- /dev/null +++ b/allianceauth/crontab/tests/test_cron.py @@ -0,0 +1,79 @@ +# myapp/tests/test_tasks.py + +import logging +from unittest.mock import patch +from django.test import TestCase +from django.db import ProgrammingError +from celery.schedules import crontab + +from allianceauth.crontab.cron import offset_cron +from allianceauth.crontab.models import CronOffset + +logger = logging.getLogger(__name__) + +class TestOffsetCron(TestCase): + + def test_offset_cron_normal(self): + """ + Test that offset_cron modifies the minute/hour fields + based on the CronOffset values when everything is normal. + """ + # We'll create a mock CronOffset instance + mock_offset = CronOffset(minute=0.5, hour=0.5) + + # Our initial crontab schedule + original_schedule = crontab( + minute=[0, 5, 55], + hour=[0, 3, 23], + day_of_month='*', + month_of_year='*', + day_of_week='*' + ) + + # Patch CronOffset.get_solo to return our mock offset + with patch('allianceauth.crontab.models.CronOffset.get_solo', return_value=mock_offset): + new_schedule = offset_cron(original_schedule) + + # Check the new minute/hour + # minute 0 -> 0 + round(60 * 0.5) = 30 % 60 = 30 + # minute 5 -> 5 + 30 = 35 % 60 = 35 + # minute 55 -> 55 + 30 = 85 % 60 = 25 --> sorted => 25,30,35 + self.assertEqual(new_schedule._orig_minute, '25,30,35') + + # hour 0 -> 0 + round(24 * 0.5) = 12 % 24 = 12 + # hour 3 -> 3 + 12 = 15 % 24 = 15 + # hour 23 -> 23 + 12 = 35 % 24 = 11 --> sorted => 11,12,15 + self.assertEqual(new_schedule._orig_hour, '11,12,15') + + # Check that other fields are unchanged + self.assertEqual(new_schedule._orig_day_of_month, '*') + self.assertEqual(new_schedule._orig_month_of_year, '*') + self.assertEqual(new_schedule._orig_day_of_week, '*') + + def test_offset_cron_programming_error(self): + """ + Test that if a ProgrammingError is raised (e.g. before migrations), + offset_cron just returns the original schedule. + """ + original_schedule = crontab(minute=[0, 15, 30], hour=[1, 2, 3]) + + # Force get_solo to raise ProgrammingError + with patch('allianceauth.crontab.models.CronOffset.get_solo', side_effect=ProgrammingError()): + new_schedule = offset_cron(original_schedule) + + # Should return the original schedule unchanged + self.assertEqual(new_schedule, original_schedule) + + def test_offset_cron_unexpected_exception(self): + """ + Test that if any other exception is raised, offset_cron + also returns the original schedule, and logs the error. + """ + original_schedule = crontab(minute='0', hour='0') + + # Force get_solo to raise a generic Exception + with patch('allianceauth.crontab.models.CronOffset.get_solo', side_effect=Exception("Something bad")): + new_schedule = offset_cron(original_schedule) + + # Should return the original schedule unchanged + self.assertEqual(new_schedule, original_schedule) diff --git a/allianceauth/crontab/tests/test_models.py b/allianceauth/crontab/tests/test_models.py new file mode 100644 index 00000000..9f7a63d4 --- /dev/null +++ b/allianceauth/crontab/tests/test_models.py @@ -0,0 +1,66 @@ +import math +from unittest.mock import patch +from django.test import TestCase + +from allianceauth.framework.models import CronOffset # adjust path as needed +from solo.models import SingletonModel + + +class CronOffsetModelTest(TestCase): + def test_cron_offset_is_singleton(self): + """ + Test that CronOffset is indeed a singleton and that + multiple calls to get_solo() return the same instance. + """ + offset1 = CronOffset.get_solo() + offset2 = CronOffset.get_solo() + + # They should be the exact same object in memory + self.assertEqual(offset1.pk, offset2.pk) + self.assertIs(offset1, offset2) + + def test_default_values_random(self): + """ + Test that the default values are set via random_default() when + no explicit value is provided. We'll patch 'random.random' to + produce predictable output. + """ + with patch('allianceauth.framework.models.random', return_value=0.1234): + # Force creation of a new CronOffset by clearing the existing one + CronOffset.objects.all().delete() + + offset = CronOffset.get_solo() # This triggers creation + + # All fields should be 0.1234, because we patched random() + self.assertAlmostEqual(offset.minute, 0.1234) + self.assertAlmostEqual(offset.hour, 0.1234) + self.assertAlmostEqual(offset.day_of_month, 0.1234) + self.assertAlmostEqual(offset.month_of_year, 0.1234) + self.assertAlmostEqual(offset.day_of_week, 0.1234) + + def test_update_offset_values(self): + """ + Test that we can update the offsets and retrieve them. + """ + offset = CronOffset.get_solo() + offset.minute = 0.5 + offset.hour = 0.25 + offset.day_of_month = 0.75 + offset.month_of_year = 0.99 + offset.day_of_week = 0.33 + offset.save() + + # Retrieve again to ensure changes persist + saved_offset = CronOffset.get_solo() + self.assertEqual(saved_offset.minute, 0.5) + self.assertEqual(saved_offset.hour, 0.25) + self.assertEqual(saved_offset.day_of_month, 0.75) + self.assertEqual(saved_offset.month_of_year, 0.99) + self.assertEqual(saved_offset.day_of_week, 0.33) + + def test_str_representation(self): + """ + Verify the __str__ method returns 'Cron Offsets'. + """ + offset = CronOffset.get_solo() + self.assertEqual(str(offset), "Cron Offsets") diff --git a/allianceauth/project_template/project_name/settings/base.py b/allianceauth/project_template/project_name/settings/base.py index 77897c99..bfe6e8ee 100644 --- a/allianceauth/project_template/project_name/settings/base.py +++ b/allianceauth/project_template/project_name/settings/base.py @@ -43,6 +43,7 @@ INSTALLED_APPS = [ 'allianceauth.theme.flatly', 'allianceauth.theme.materia', "allianceauth.custom_css", + 'allianceauth.crontab', ] SECRET_KEY = "wow I'm a really bad default secret key" From a66aa6de80c17ad6273ad3fb35d48d84cc0496ff Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Sun, 29 Dec 2024 22:19:51 +1000 Subject: [PATCH 07/10] rename submodule properly --- allianceauth/crontab/tests/test_models.py | 6 ++-- .../framework/migrations/0001_initial.py | 29 ------------------- 2 files changed, 2 insertions(+), 33 deletions(-) delete mode 100644 allianceauth/framework/migrations/0001_initial.py diff --git a/allianceauth/crontab/tests/test_models.py b/allianceauth/crontab/tests/test_models.py index 9f7a63d4..f5f2aa50 100644 --- a/allianceauth/crontab/tests/test_models.py +++ b/allianceauth/crontab/tests/test_models.py @@ -1,9 +1,7 @@ -import math from unittest.mock import patch from django.test import TestCase -from allianceauth.framework.models import CronOffset # adjust path as needed -from solo.models import SingletonModel +from allianceauth.crontab.models import CronOffset class CronOffsetModelTest(TestCase): @@ -25,7 +23,7 @@ class CronOffsetModelTest(TestCase): no explicit value is provided. We'll patch 'random.random' to produce predictable output. """ - with patch('allianceauth.framework.models.random', return_value=0.1234): + with patch('allianceauth.crontab.models.random', return_value=0.1234): # Force creation of a new CronOffset by clearing the existing one CronOffset.objects.all().delete() diff --git a/allianceauth/framework/migrations/0001_initial.py b/allianceauth/framework/migrations/0001_initial.py deleted file mode 100644 index 4888ceda..00000000 --- a/allianceauth/framework/migrations/0001_initial.py +++ /dev/null @@ -1,29 +0,0 @@ -# Generated by Django 4.2.16 on 2024-12-29 05:02 - -import allianceauth.framework.models -from django.db import migrations, models - - -class Migration(migrations.Migration): - - initial = True - - dependencies = [ - ] - - operations = [ - migrations.CreateModel( - name='CronOffset', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('minute', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Minute Offset')), - ('hour', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Hour Offset')), - ('day_of_month', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Day of Month Offset')), - ('month_of_year', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Month Of Year Offset')), - ('day_of_week', models.FloatField(default=allianceauth.framework.models.random_default, verbose_name='Day of Week Offset')), - ], - options={ - 'verbose_name': 'Cron Offsets', - }, - ), - ] From 6f2f39d7fa9c5d8999c276e793778968f7284c25 Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Mon, 30 Dec 2024 18:12:11 +1000 Subject: [PATCH 08/10] shift to custom Scheduler --- allianceauth/apps.py | 46 -------------- allianceauth/crontab/schedulers.py | 63 +++++++++++++++++++ allianceauth/crontab/tests/test_models.py | 1 - .../tests/{test_cron.py => test_utils.py} | 3 +- allianceauth/crontab/{cron.py => utils.py} | 1 + .../project_name/settings/base.py | 28 ++++++++- 6 files changed, 92 insertions(+), 50 deletions(-) create mode 100644 allianceauth/crontab/schedulers.py rename allianceauth/crontab/tests/{test_cron.py => test_utils.py} (98%) rename allianceauth/crontab/{cron.py => utils.py} (99%) diff --git a/allianceauth/apps.py b/allianceauth/apps.py index c2397c9b..1191fed9 100644 --- a/allianceauth/apps.py +++ b/allianceauth/apps.py @@ -1,5 +1,4 @@ from django.apps import AppConfig -from celery.schedules import crontab class AllianceAuthConfig(AppConfig): @@ -7,48 +6,3 @@ class AllianceAuthConfig(AppConfig): def ready(self) -> None: import allianceauth.checks # noqa - from django_celery_beat.models import CrontabSchedule, PeriodicTask - from allianceauth.crontab.cron import offset_cron - - PeriodicTask.objects.update_or_create( - name='esi_cleanup_callbackredirect', - defaults={ - 'task': 'esi.tasks.cleanup_callbackredirect', - 'crontab': CrontabSchedule.objects.get_or_create(minute='0', hour='0', day_of_week='*', day_of_month='*', month_of_year='*', timezone='UTC')[0], - }, - ) - PeriodicTask.objects.update_or_create( - name='esi_cleanup_token', - defaults={ - 'task': 'esi.tasks.cleanup_token', - 'crontab': CrontabSchedule.objects.get_or_create(minute='0', hour='0', day_of_week='*', day_of_month='*', month_of_year='*', timezone='UTC')[0], - }, - ) - - z = CrontabSchedule.from_schedule(offset_cron(crontab(minute='0', hour='*/6'))) - PeriodicTask.objects.update_or_create( - name='run_model_update', - defaults={ - 'task': 'allianceauth.eveonline.tasks.run_model_update', - 'crontab': CrontabSchedule.objects.get_or_create( # Convert the offsetted cron into a DB object - minute=z.minute, hour=z.hour, day_of_week=z.day_of_week, day_of_month=z.day_of_month, month_of_year=z.month_of_year, timezone=z.timezone)[0], - }, - ) - - z = CrontabSchedule.from_schedule(offset_cron(crontab(minute='0', hour='*/4'))) - PeriodicTask.objects.update_or_create( - name='check_all_character_ownership', - defaults={ - 'task': 'allianceauth.authentication.tasks.check_all_character_ownership', - 'crontab': CrontabSchedule.objects.get_or_create( # Convert the offsetted cron into a DB object - minute=z.minute, hour=z.hour, day_of_week=z.day_of_week, day_of_month=z.day_of_month, month_of_year=z.month_of_year, timezone=z.timezone)[0], - }, - ) - PeriodicTask.objects.update_or_create( - name='analytics_daily_stats', - defaults={ - 'task': 'allianceauth.analytics.tasks.analytics_daily_stats', - 'crontab': CrontabSchedule.objects.get_or_create( - minute='0', hour='12', day_of_week='*', day_of_month='*', month_of_year='*', timezone='UTC')[0], - }, - ) diff --git a/allianceauth/crontab/schedulers.py b/allianceauth/crontab/schedulers.py new file mode 100644 index 00000000..0b36eead --- /dev/null +++ b/allianceauth/crontab/schedulers.py @@ -0,0 +1,63 @@ +from django.core.exceptions import ObjectDoesNotExist +from django_celery_beat.schedulers import ( + DatabaseScheduler +) +from django_celery_beat.models import CrontabSchedule +from django.db.utils import OperationalError, ProgrammingError + +from celery import schedules +from celery.utils.log import get_logger + +from allianceauth.crontab.models import CronOffset +from allianceauth.crontab.utils import offset_cron + +logger = get_logger(__name__) + + +class OffsetDatabaseScheduler(DatabaseScheduler): + """ + Customization of Django Celery Beat, Database Scheduler + Takes the Celery Schedule from local.py and applies our AA Framework Cron Offset, if apply_offset is true + Otherwise it passes it through as normal + """ + def update_from_dict(self, mapping): + s = {} + + try: + cron_offset = CronOffset.get_solo() + except (OperationalError, ProgrammingError, ObjectDoesNotExist) as exc: + # This is just incase we haven't migrated yet or something + logger.warning( + "OffsetDatabaseScheduler: Could not fetch CronOffset (%r). " + "Defering to DatabaseScheduler", + exc + ) + return super().update_from_dict(mapping) + + for name, entry_fields in mapping.items(): + try: + apply_offset = entry_fields.pop("apply_offset", False) + entry = self.Entry.from_entry(name, app=self.app, **entry_fields) + + if entry.model.enabled and apply_offset: + schedule_obj = entry.schedule + if isinstance(schedule_obj, schedules.crontab): + offset_cs = CrontabSchedule.from_schedule(offset_cron(schedule_obj)) + offset_cs, created = CrontabSchedule.objects.get_or_create( + minute=offset_cs.minute, + hour=offset_cs.hour, + day_of_month=offset_cs.day_of_month, + month_of_year=offset_cs.month_of_year, + day_of_week=offset_cs.day_of_week, + timezone=offset_cs.timezone, + ) + entry.model.crontab = offset_cs + entry.model.save() + logger.debug(f"Offset applied for '{name}' due to 'apply_offset' = True.") + + s[name] = entry + + except Exception as e: + logger.exception("Error updating schedule for %s: %r", name, e) + + self.schedule.update(s) diff --git a/allianceauth/crontab/tests/test_models.py b/allianceauth/crontab/tests/test_models.py index f5f2aa50..cb92e667 100644 --- a/allianceauth/crontab/tests/test_models.py +++ b/allianceauth/crontab/tests/test_models.py @@ -15,7 +15,6 @@ class CronOffsetModelTest(TestCase): # They should be the exact same object in memory self.assertEqual(offset1.pk, offset2.pk) - self.assertIs(offset1, offset2) def test_default_values_random(self): """ diff --git a/allianceauth/crontab/tests/test_cron.py b/allianceauth/crontab/tests/test_utils.py similarity index 98% rename from allianceauth/crontab/tests/test_cron.py rename to allianceauth/crontab/tests/test_utils.py index c9a2a970..48592c99 100644 --- a/allianceauth/crontab/tests/test_cron.py +++ b/allianceauth/crontab/tests/test_utils.py @@ -6,11 +6,12 @@ from django.test import TestCase from django.db import ProgrammingError from celery.schedules import crontab -from allianceauth.crontab.cron import offset_cron +from allianceauth.crontab.utils import offset_cron from allianceauth.crontab.models import CronOffset logger = logging.getLogger(__name__) + class TestOffsetCron(TestCase): def test_offset_cron_normal(self): diff --git a/allianceauth/crontab/cron.py b/allianceauth/crontab/utils.py similarity index 99% rename from allianceauth/crontab/cron.py rename to allianceauth/crontab/utils.py index bbeecddd..150664ca 100644 --- a/allianceauth/crontab/cron.py +++ b/allianceauth/crontab/utils.py @@ -3,6 +3,7 @@ import logging from allianceauth.crontab.models import CronOffset from django.db import ProgrammingError + logger = logging.getLogger(__name__) diff --git a/allianceauth/project_template/project_name/settings/base.py b/allianceauth/project_template/project_name/settings/base.py index bfe6e8ee..19bdbe77 100644 --- a/allianceauth/project_template/project_name/settings/base.py +++ b/allianceauth/project_template/project_name/settings/base.py @@ -50,8 +50,32 @@ SECRET_KEY = "wow I'm a really bad default secret key" # Celery configuration BROKER_URL = 'redis://localhost:6379/0' -CELERYBEAT_SCHEDULER = "django_celery_beat.schedulers.DatabaseScheduler" -CELERYBEAT_SCHEDULE = {} +CELERYBEAT_SCHEDULER = "allianceauth.crontab.schedulers.OffsetDatabaseScheduler" +CELERYBEAT_SCHEDULE = { + 'esi_cleanup_callbackredirect': { + 'task': 'esi.tasks.cleanup_callbackredirect', + 'schedule': crontab(minute='0', hour='*/4'), + }, + 'esi_cleanup_token': { + 'task': 'esi.tasks.cleanup_token', + 'schedule': crontab(minute='0', hour='0'), + }, + 'run_model_update': { + 'task': 'allianceauth.eveonline.tasks.run_model_update', + 'schedule': crontab(minute='0', hour="*/6"), + 'apply_offset': True + }, + 'check_all_character_ownership': { + 'task': 'allianceauth.authentication.tasks.check_all_character_ownership', + 'schedule': crontab(minute='0', hour='*/4'), + 'apply_offset': True + }, + 'analytics_daily_stats': { + 'task': 'allianceauth.analytics.tasks.analytics_daily_stats', + 'schedule': crontab(minute='0', hour='2'), + } +} + # Build paths inside the project like this: os.path.join(BASE_DIR, ...) PROJECT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) From 156e7c891eac17373ee1dcf279952bd0450afe04 Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Mon, 30 Dec 2024 18:57:09 +1000 Subject: [PATCH 09/10] document cron offsetting --- docs/development/tech_docu/celery.md | 51 ++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/docs/development/tech_docu/celery.md b/docs/development/tech_docu/celery.md index 3f39d299..b9571170 100644 --- a/docs/development/tech_docu/celery.md +++ b/docs/development/tech_docu/celery.md @@ -123,6 +123,7 @@ Example setting: CELERYBEAT_SCHEDULE['structures_update_all_structures'] = { 'task': 'structures.tasks.update_all_structures', 'schedule': crontab(minute='*/30'), + 'apply_offset': True, } ``` @@ -130,6 +131,7 @@ CELERYBEAT_SCHEDULE['structures_update_all_structures'] = { - `'task'`: Name of your task (full path) - `'schedule'`: Schedule definition (see Celery documentation on [Periodic Tasks](https://docs.celeryproject.org/en/latest/userguide/periodic-tasks.html) for details) +- `'apply_offset'`: Boolean, Apply a Delay unique to the install, in order to reduce impact on ESI. See [Apply Offset](#apply-offset) ## How can I use priorities for tasks? @@ -174,9 +176,54 @@ Large numbers of installs running the same crontab (ie. `0 * * * *`) can all sla Consider Artificially smoothing out your tasks with a few methods -### Offset Crontabs +### Apply Offset -Avoid running your tasks on the hour or other nice neat human numbers, consider 23 minutes on the hour instead of at zero (`28 * * * *`) +`allianceauth.crontab` contains a series of Offsets stored in the DB that are both static for an install, but random across all AA installs. + +This enables us to spread our load on ESI (or other external resources) across a greater window, making it unlikely that two installs will hit ESI at the same time. + +Tasks defined in local.py, can have `'apply_offset': True` added to their Task Definition + +```python +CELERYBEAT_SCHEDULE['taskname'] = { + 'task': 'module.tasks.task', + 'schedule': crontab(minute='*/30'), + 'apply_offset': True, +} +``` + +Tasks added to directly to Django Celery Beat Models (Using a Management Task etc) can pass their Cron Schedule through offset_cron(crontab) + +```{eval-rst} +.. automodule:: allianceauth.crontab.utils + :members: + :undoc-members: +``` + +```python +from django_celery_beat.models import CrontabSchedule, PeriodicTask +from celery.schedules import crontab + +schedule = CrontabSchedule.from_schedule(offset_cron(crontab(minute='0', hour='0'))) + +schedule, created = CrontabSchedule.objects.get_or_create( + minute=schedule.minute, + hour=schedule.hour, + day_of_month=schedule.day_of_month, + month_of_year=schedule.month_of_year, + day_of_week=schedule.day_of_week, + timezone=schedule.timezone, +) + +PeriodicTask.objects.update_or_create( + task='module.tasks.task', + defaults={ + 'crontab': schedule, + 'name': 'task name', + 'enabled': True + } +) +``` ### Subset Tasks From 86abc4f1692d33e9c7ae4fa6f5dba6eec9b4143e Mon Sep 17 00:00:00 2001 From: Ariel Rin Date: Fri, 10 Jan 2025 12:04:44 +0000 Subject: [PATCH 10/10] Update file __init__.py --- allianceauth/framework/migrations/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 allianceauth/framework/migrations/__init__.py diff --git a/allianceauth/framework/migrations/__init__.py b/allianceauth/framework/migrations/__init__.py deleted file mode 100644 index e69de29b..00000000