Skip to content

Commit 86c0473

Browse files
committed
feat: configurable window on scheduled notify_credentials jobs
`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
1 parent 43a3a2a commit 86c0473

File tree

4 files changed

+23
-2
lines changed

4 files changed

+23
-2
lines changed

cms/envs/common.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,6 +2520,8 @@
25202520
CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:8005'
25212521
CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:8005'
25222522
CREDENTIALS_SERVICE_USERNAME = 'credentials_service_user'
2523+
# time between scheduled runs, in seconds
2524+
NOTIFY_CREDENTIALS_FREQUENCY = 14400
25232525

25242526
ANALYTICS_DASHBOARD_URL = 'http://localhost:18110/courses'
25252527
ANALYTICS_DASHBOARD_NAME = 'Your Platform Name Here Insights'

lms/envs/common.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4331,6 +4331,8 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
43314331

43324332
CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:8005'
43334333
CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:8005'
4334+
# time between scheduled runs, in seconds
4335+
NOTIFY_CREDENTIALS_FREQUENCY = 14400
43344336

43354337
COMMENTS_SERVICE_URL = 'http://localhost:18080'
43364338
COMMENTS_SERVICE_KEY = 'password'

openedx/core/djangoapps/credentials/management/commands/notify_credentials.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from datetime import datetime, timedelta
1515
import dateutil.parser
16+
from django.conf import settings
1617
from django.core.management.base import BaseCommand, CommandError
1718
from opaque_keys import InvalidKeyError
1819
from opaque_keys.edx.keys import CourseKey
@@ -157,8 +158,9 @@ def handle(self, *args, **options):
157158
options = self.get_args_from_database()
158159

159160
if options["auto"]:
161+
run_frequency = settings.NOTIFY_CREDENTIALS_FREQUENCY
160162
options["end_date"] = datetime.now().replace(minute=0, second=0, microsecond=0)
161-
options["start_date"] = options["end_date"] - timedelta(hours=4)
163+
options["start_date"] = options["end_date"] - timedelta(seconds=run_frequency)
162164

163165
log.info(
164166
f"notify_credentials starting, dry-run={options['dry_run']}, site={options['site']}, "

openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from django.core.management import call_command
99
from django.core.management.base import CommandError
10-
from django.test import TestCase, override_settings # lint-amnesty, pylint: disable=unused-import
10+
from django.test import TestCase, override_settings
1111
from freezegun import freeze_time
1212

1313
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):
125125

126126
@freeze_time(datetime(2017, 5, 1, 4))
127127
def test_auto_execution(self, mock_task):
128+
"""Verify that an automatic execution designed for scheduled windows works correctly"""
128129
self.expected_options['auto'] = True
129130
self.expected_options['start_date'] = datetime(2017, 5, 1, 0, 0)
130131
self.expected_options['end_date'] = datetime(2017, 5, 1, 4, 0)
@@ -133,6 +134,20 @@ def test_auto_execution(self, mock_task):
133134
assert mock_task.called
134135
assert mock_task.call_args[0][0] == self.expected_options
135136

137+
138+
@override_settings(NOTIFY_CREDENTIALS_FREQUENCY=3600)
139+
@freeze_time(datetime(2017, 5, 1, 4))
140+
def test_auto_execution_different_schedule(self, mock_task):
141+
"""Verify that an automatic execution designed for scheduled windows
142+
works correctly if the window frequency has been changed"""
143+
self.expected_options["auto"] = True
144+
self.expected_options["start_date"] = datetime(2017, 5, 1, 3, 0)
145+
self.expected_options["end_date"] = datetime(2017, 5, 1, 4, 0)
146+
147+
call_command(Command(), "--auto")
148+
assert mock_task.called
149+
assert mock_task.call_args[0][0] == self.expected_options
150+
136151
def test_date_args(self, mock_task):
137152
self.expected_options['start_date'] = datetime(2017, 1, 31, 0, 0, tzinfo=timezone.utc)
138153
call_command(Command(), '--start-date', '2017-01-31')

0 commit comments

Comments
 (0)