Skip to content

Commit

Permalink
feat: increase frequency of scheduled notify credentials (openedx#36180)
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
deborahgu authored and leoaulasneo98 committed Jan 30, 2025
1 parent 3fba328 commit c7e4969
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 2 deletions.
2 changes: 2 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 2 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']}, "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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')
Expand Down

0 comments on commit c7e4969

Please sign in to comment.