Ali sr/7068 figures job optimization

figures/management/commands/populate_figures_metrics.py

@@ -45,6 +45,10 @@ def add_arguments(self, parser):
                             action='store_true',
                             default=False,
                             help='Run just the MAU pipeline')
+        parser.add_argument('--active-courses',
+                            action='store_true',
+                            default=False,
+                            help='Populate only active courses')
 
     def handle(self, *args, **options):
         '''
@@ -78,9 +82,9 @@ def handle(self, *args, **options):
                     experimental_populate_daily_metrics.delay(**kwargs)  # pragma: no cover
             else:
                 if options['no_delay']:
-                    populate_daily_metrics(**kwargs)
+                    populate_daily_metrics(active_courses=options['active_courses'], **kwargs)
                 else:
-                    populate_daily_metrics.delay(**kwargs)  # pragma: no cover
+                    populate_daily_metrics.delay(active_courses=options['active_courses'], **kwargs)  # pragma: no cover
 
         # TODO: improve this message to say 'today' when options['date'] is None
         print('Management command populate_figures_metrics complete. date_for: {}'.format(

figures/sites.py

@@ -11,6 +11,8 @@
 """
 
 from __future__ import absolute_import
+
+from datetime import datetime
 from django.contrib.auth import get_user_model
 from django.contrib.sites.models import Site
 from django.conf import settings
@@ -168,17 +170,23 @@ def get_course_keys_for_sites_slugs(site_slugs):
     return [as_course_key(cid) for cid in course_ids]
 
 
-def get_course_keys_for_site(site):
+def get_course_keys_for_site(site, active_courses=False):
     if figures.helpers.is_multisite():
         edx_orgs = EdlySubOrganization.objects.filter(lms_site=site).using(read_replica_or_default()).values_list(
             'edx_organizations', flat=True)
         org_courses = organizations.models.OrganizationCourse.objects.filter(organization__in=edx_orgs).using(
             read_replica_or_default())
 
+        if active_courses:
+            org_courses = org_courses.filter(active=True)
+
         course_ids = org_courses.values_list('course_id', flat=True)
     else:
-        course_ids = CourseOverview.objects.using(
-            read_replica_or_default()).all().values_list('id', flat=True)
+        course_ids = CourseOverview.objects.using(read_replica_or_default()).all()
+        if active_courses:
+            course_ids = course_ids.filter(end_date__gte=datetime.now())
+
+        course_ids = course_ids.values_list('id', flat=True)
 
     return [as_course_key(cid) for cid in course_ids]
 
@@ -196,16 +204,19 @@ def site_course_ids(site):
             'id', flat=True)]
 
 
-def get_courses_for_site(site):
+def get_courses_for_site(site, active_courses=False):
     """Returns the courses accessible by the user on the site
 
     This function relies on Appsembler's fork of edx-organizations
     """
     if figures.helpers.is_multisite():
-        course_keys = get_course_keys_for_site(site)
+        course_keys = get_course_keys_for_site(site, active_courses)
         courses = CourseOverview.objects.filter(id__in=course_keys).using(read_replica_or_default())
     else:
         courses = CourseOverview.objects.using(read_replica_or_default()).all()
+        if active_courses:
+            courses = courses.filter(active=True, end_date__gte=datetime.now())
+
     return courses
 
 

figures/tasks.py

@@ -167,7 +167,7 @@ def update_learners_progress_for_course(course):
 
 
 @shared_task
-def populate_daily_metrics(date_for=None, force_update=False):
+def populate_daily_metrics(date_for=None, force_update=False, active_courses=False):
     '''Populates the daily metrics models for the given date
 
     This method populates CourseDailyMetrics for all the courses in the site,
@@ -198,7 +198,7 @@ def populate_daily_metrics(date_for=None, force_update=False):
     sites_count = len(lms_sites)
     for i, site in enumerate(Site.objects.using(read_replica_or_default()).filter(id__in=lms_sites)):
         try:
-            courses = figures.sites.get_courses_for_site(site)
+            courses = figures.sites.get_courses_for_site(site, active_courses)
         except Exception:  # pylint: disable=broad-except
             courses = []
             msg = ('FIGURES:FAIL populate_daily_metrics unhandled site level'

tests/test_tasks.py

@@ -82,7 +82,7 @@ def test_populate_daily_metrics_site_level_error(transactional_db,
     error_message = dict(message=[u'expected failure'])
     assert not CourseOverview.objects.count()
 
-    def mock_get_courses_fail(site):
+    def mock_get_courses_fail(site, active_courses=False):
         raise Exception(error_message)
 
     assert SiteDailyMetrics.objects.count() == 0
@@ -92,9 +92,13 @@ def mock_get_courses_fail(site):
 
     figures.tasks.populate_daily_metrics(date_for=date_for)
 
-    last_log = caplog.records[0]
-    assert last_log.message.startswith(
-        'FIGURES:FAIL populate_daily_metrics unhandled site level exception for site')
+    error_found = False
+    for record in caplog.records:
+        if record.message.startswith('FIGURES:FAIL populate_daily_metrics unhandled site level exception for site'):
+            error_found = True
+            break
+
+    assert error_found, "Expected error message not found in logs"
 
 
 @pytest.mark.skipif(OPENEDX_RELEASE == GINKGO,
@@ -105,7 +109,7 @@ def test_populate_daily_metrics_error(transactional_db, monkeypatch):
     error_message = dict(message=[u'expected failure'])
     assert not CourseOverview.objects.count()
 
-    def mock_get_courses(site):
+    def mock_get_courses(site, active_courses=False):
         CourseOverviewFactory()
         return CourseOverview.objects.all()
 
@@ -142,7 +146,7 @@ def test_populate_daily_metrics_enrollment_data_error(transactional_db,
     error_message = dict(message=[u'expected failure'])
     assert not CourseOverview.objects.count()
 
-    def mock_get_courses(site):
+    def mock_get_courses(site, active_courses=False):
         CourseOverviewFactory()
         return CourseOverview.objects.all()
 
@@ -167,10 +171,15 @@ def mock_update_learners_progress_for_course(course):
         figures.tasks, 'update_enrollment_data', mock_update_enrollment_data_fails)
     monkeypatch.setattr(
         figures.tasks, 'update_learners_progress_for_course', mock_update_learners_progress_for_course)
-    figures.tasks.populate_daily_metrics(date_for=date_for)
-    last_log = caplog.records[-1]
-    assert last_log.message.startswith(
-        'FIGURES:FAIL figures.tasks update_enrollment_data')
+    figures.tasks.populate_daily_metrics(date_for=date_for, active_courses=False)
+
+    error_found = False
+    for record in caplog.records:
+        if record.message.startswith('FIGURES:FAIL figures.tasks update_enrollment_data'):
+            error_found = True
+            break
+
+    assert error_found, "Expected error message not found in logs"
 
 
 @pytest.mark.skipif(OPENEDX_RELEASE == GINKGO,