From 1fe67d3f6b40233791d4599bae28df8c0ac91c4d Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Mon, 27 Jan 2025 14:26:00 -0800 Subject: [PATCH] feat: increase frequency of scheduled notify credentials (#36180) `notify_credentials` has 2 ways of running. 1. The manual, one-off method which uses `--args_from_database` to specify what should be sent. 2. [The automated method](https://github.com/openedx/edx-platform/blob/7316111b35c8db0b93665e00aa4071685d772ab3/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py#L157-L159), which runs on a regular schedule, to catch anything which fell through the cracks. The automated method does a certain amount of time/date math in order to calculate the entry point of the window based on the current runtime. This is, I assume, why it has some hardcoded logic; it's not at all simple to have a `cron`-run management command running on a regular cadence that can do the same time logic. ```py if options['auto']: options['end_date'] = datetime.now().replace(minute=0, second=0, microsecond=0) options['start_date'] = options['end_date'] - timedelta(hours=4) ``` However, it is not ideal that the actual time window of 4 hours is hardcoded directly into `edx-platform`. This fix * creates an overridable `NOTIFY_CREDENTIALS_FREQUENCY` that defaults to 14400 seconds (4 hours). * pulls that frequency into the quoted code Using seconds allows maximum flexibility. FIXES: APER-3383 --- cms/envs/common.py | 2 ++ lms/envs/common.py | 2 ++ .../management/commands/notify_credentials.py | 4 +++- .../commands/tests/test_notify_credentials.py | 16 +++++++++++++++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 591247388a9d..0919834a9b64 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2520,6 +2520,8 @@ CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:8005' CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:8005' CREDENTIALS_SERVICE_USERNAME = 'credentials_service_user' +# time between scheduled runs, in seconds +NOTIFY_CREDENTIALS_FREQUENCY = 14400 ANALYTICS_DASHBOARD_URL = 'http://localhost:18110/courses' ANALYTICS_DASHBOARD_NAME = 'Your Platform Name Here Insights' diff --git a/lms/envs/common.py b/lms/envs/common.py index 76127d062d81..c57a08b617db 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4331,6 +4331,8 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:8005' CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:8005' +# time between scheduled runs, in seconds +NOTIFY_CREDENTIALS_FREQUENCY = 14400 COMMENTS_SERVICE_URL = 'http://localhost:18080' COMMENTS_SERVICE_KEY = 'password' diff --git a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py index 1b7ff682baf6..f79ead24d9b0 100644 --- a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py @@ -13,6 +13,7 @@ from datetime import datetime, timedelta import dateutil.parser +from django.conf import settings from django.core.management.base import BaseCommand, CommandError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -157,8 +158,9 @@ def handle(self, *args, **options): options = self.get_args_from_database() if options["auto"]: + run_frequency = settings.NOTIFY_CREDENTIALS_FREQUENCY options["end_date"] = datetime.now().replace(minute=0, second=0, microsecond=0) - options["start_date"] = options["end_date"] - timedelta(hours=4) + options["start_date"] = options["end_date"] - timedelta(seconds=run_frequency) log.info( f"notify_credentials starting, dry-run={options['dry_run']}, site={options['site']}, " diff --git a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py index e50173001fd3..f7bed3446aca 100644 --- a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py @@ -7,7 +7,7 @@ from django.core.management import call_command from django.core.management.base import CommandError -from django.test import TestCase, override_settings # lint-amnesty, pylint: disable=unused-import +from django.test import TestCase, override_settings from freezegun import freeze_time from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory, CourseFactory, CourseRunFactory @@ -125,6 +125,7 @@ def test_multiple_programs_uuid_args(self, mock_get_programs, mock_task): @freeze_time(datetime(2017, 5, 1, 4)) def test_auto_execution(self, mock_task): + """Verify that an automatic execution designed for scheduled windows works correctly""" self.expected_options['auto'] = True self.expected_options['start_date'] = datetime(2017, 5, 1, 0, 0) self.expected_options['end_date'] = datetime(2017, 5, 1, 4, 0) @@ -133,6 +134,19 @@ def test_auto_execution(self, mock_task): assert mock_task.called assert mock_task.call_args[0][0] == self.expected_options + @override_settings(NOTIFY_CREDENTIALS_FREQUENCY=3600) + @freeze_time(datetime(2017, 5, 1, 4)) + def test_auto_execution_different_schedule(self, mock_task): + """Verify that an automatic execution designed for scheduled windows + works correctly if the window frequency has been changed""" + self.expected_options["auto"] = True + self.expected_options["start_date"] = datetime(2017, 5, 1, 3, 0) + self.expected_options["end_date"] = datetime(2017, 5, 1, 4, 0) + + call_command(Command(), "--auto") + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + def test_date_args(self, mock_task): self.expected_options['start_date'] = datetime(2017, 1, 31, 0, 0, tzinfo=timezone.utc) call_command(Command(), '--start-date', '2017-01-31')