Merge pull request #14700 from edx/neem/backfill-mgt

backfill_grades management command

lms/djangoapps/grades/management/commands/compute_grades.py

+"""
+Command to compute all grades for specified courses.
+"""
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import logging
+
+from django.core.management.base import BaseCommand
+import six
+
+from openedx.core.lib.command_utils import (
+    get_mutually_exclusive_required_option,
+    parse_course_keys,
+)
+from student.models import CourseEnrollment
+from xmodule.modulestore.django import modulestore
+
+from ... import tasks
+
+
+log = logging.getLogger(__name__)
+
+
+class Command(BaseCommand):
+    """
+    Example usage:
+        $ ./manage.py lms compute_grades --all_courses --settings=devstack
+        $ ./manage.py lms compute_grades 'edX/DemoX/Demo_Course' --settings=devstack
+    """
+    args = '<course_id course_id ...>'
+    help = 'Computes grade values for all learners in specified courses.'
+
+    def add_arguments(self, parser):
+        """
+        Entry point for subclassed commands to add custom arguments.
+        """
+        parser.add_argument(
+            '--courses',
+            dest='courses',
+            nargs='+',
+            help='List of (space separated) courses that need grades computed.',
+        )
+        parser.add_argument(
+            '--all_courses',
+            help='Compute grades for all courses.',
+            action='store_true',
+            default=False,
+        )
+        parser.add_argument(
+            '--routing_key',
+            dest='routing_key',
+            help='Celery routing key to use.',
+        )
+        parser.add_argument(
+            '--batch_size',
+            help='Maximum number of students to calculate grades for, per celery task.',
+            default=100,
+            type=int,
+        )
+        parser.add_argument(
+            '--start_index',
+            help='Offset from which to start processing enrollments.',
+            default=0,
+            type=int,
+        )
+
+    def handle(self, *args, **options):
+        self._set_log_level(options)
+
+        for course_key in self._get_course_keys(options):
+            self.enqueue_compute_grades_for_course_tasks(course_key, options)
+
+    def enqueue_compute_grades_for_course_tasks(self, course_key, options):
+        """
+        Enqueue celery tasks to compute and persist all grades for the
+        specified course, in batches.
+        """
+        enrollment_count = CourseEnrollment.objects.filter(course_id=course_key).count()
+        if enrollment_count == 0:
+            log.warning("No enrollments found for {}".format(course_key))
+        for offset in six.moves.range(options['start_index'], enrollment_count, options['batch_size']):
+            # If the number of enrollments increases after the tasks are
+            # created, the most recent enrollments may not get processed.
+            # This is an acceptable limitation for our known use cases.
+            task_options = {'routing_key': options['routing_key']} if options.get('routing_key') else {}
+            kwargs = {
+                'course_key': six.text_type(course_key),
+                'offset': offset,
+                'batch_size': options['batch_size'],
+            }
+            result = tasks.compute_grades_for_course.apply_async(
+                kwargs=kwargs,
+                options=task_options,
+            )
+            log.info("Persistent grades: Created {task_name}[{task_id}] with arguments {kwargs}".format(
+                task_name=tasks.compute_grades_for_course.name,
+                task_id=result.task_id,
+                kwargs=kwargs,
+            ))
+
+    def _get_course_keys(self, options):
+        """
+        Return a list of courses that need scores computed.
+        """
+        courses_mode = get_mutually_exclusive_required_option(options, 'courses', 'all_courses')
+        if courses_mode == 'all_courses':
+            course_keys = [course.id for course in modulestore().get_course_summaries()]
+        else:
+            course_keys = parse_course_keys(options['courses'])
+        return course_keys
+
+    def _set_log_level(self, options):
+        """
+        Sets logging levels for this module and the block structure
+        cache module, based on the given the options.
+        """
+        if options.get('verbosity') == 0:
+            log_level = logging.WARNING
+        elif options.get('verbosity') >= 1:
+            log_level = logging.INFO
+        log.setLevel(log_level)

lms/djangoapps/grades/management/commands/tests/test_compute_grades.py

+"""
+Tests for compute_grades management command.
+"""
+
+# pylint: disable=protected-access
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import ddt
+from django.contrib.auth import get_user_model
+from django.core.management import CommandError, call_command
+from mock import patch
+import six
+
+from student.models import CourseEnrollment
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+from lms.djangoapps.grades.management.commands import compute_grades
+
+
+@ddt.ddt
+class TestComputeGrades(SharedModuleStoreTestCase):
+    """
+    Tests compute_grades management command.
+    """
+    num_users = 3
+    num_courses = 5
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestComputeGrades, cls).setUpClass()
+        User = get_user_model()  # pylint: disable=invalid-name
+        cls.command = compute_grades.Command()
+
+        cls.courses = [CourseFactory.create() for _ in range(cls.num_courses)]
+        cls.course_keys = [six.text_type(course.id) for course in cls.courses]
+        cls.users = [User.objects.create(username='user{}'.format(idx)) for idx in range(cls.num_users)]
+
+        for user in cls.users:
+            for course in cls.courses:
+                CourseEnrollment.enroll(user, course.id)
+
+    def test_select_all_courses(self):
+        courses = self.command._get_course_keys({'all_courses': True})
+        self.assertEqual(
+            sorted(six.text_type(course) for course in courses),
+            self.course_keys,
+        )
+
+    def test_specify_courses(self):
+        courses = self.command._get_course_keys({'courses': [self.course_keys[0], self.course_keys[1], 'd/n/e']})
+        self.assertEqual(
+            [six.text_type(course) for course in courses],
+            [self.course_keys[0], self.course_keys[1], 'd/n/e'],
+        )
+
+    def test_selecting_invalid_course(self):
+        with self.assertRaises(CommandError):
+            self.command._get_course_keys({'courses': [self.course_keys[0], self.course_keys[1], 'badcoursekey']})
+
+    @patch('lms.djangoapps.grades.tasks.compute_grades_for_course')
+    def test_tasks_fired(self, mock_task):
+        call_command(
+            'compute_grades',
+            '--routing_key=key',
+            '--batch_size=2',
+            '--courses',
+            self.course_keys[0],
+            self.course_keys[3],
+            'd/n/e'  # No tasks created for nonexistent course, because it has no enrollments
+        )
+        self.assertEqual(
+            mock_task.apply_async.call_args_list,
+            [
+                ({
+                    'options': {'routing_key': 'key'},
+                    'kwargs': {'course_key': self.course_keys[0], 'batch_size': 2, 'offset': 0}
+                },),
+                ({
+                    'options': {'routing_key': 'key'},
+                    'kwargs': {'course_key': self.course_keys[0], 'batch_size': 2, 'offset': 2}
+                },),
+                ({
+                    'options': {'routing_key': 'key'},
+                    'kwargs': {'course_key': self.course_keys[3], 'batch_size': 2, 'offset': 0}
+                },),
+                ({
+                    'options': {'routing_key': 'key'},
+                    'kwargs': {'course_key': self.course_keys[3], 'batch_size': 2, 'offset': 2}
+                },),
+            ],
+        )

lms/djangoapps/grades/management/commands/tests/test_reset_grades.py

@@ -302,17 +302,17 @@ class TestResetGrades(TestCase):
             self.command.handle(delete=True, all_courses=True, db_table="not course or subsection")
 
     def test_no_run_mode(self):
-        with self.assertRaisesMessage(CommandError, 'Either --delete or --dry_run must be specified.'):
+        with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --delete, --dry_run'):
             self.command.handle(all_courses=True)
 
     def test_both_run_modes(self):
-        with self.assertRaisesMessage(CommandError, 'Both --delete and --dry_run cannot be specified.'):
+        with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --delete, --dry_run'):
             self.command.handle(all_courses=True, dry_run=True, delete=True)
 
     def test_no_course_mode(self):
-        with self.assertRaisesMessage(CommandError, 'Either --courses or --all_courses must be specified.'):
+        with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --courses, --all_courses'):
             self.command.handle(dry_run=True)
 
     def test_both_course_modes(self):
-        with self.assertRaisesMessage(CommandError, 'Both --courses and --all_courses cannot be specified.'):
+        with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --courses, --all_courses'):
             self.command.handle(dry_run=True, all_courses=True, courses=['some/course/key'])

lms/djangoapps/grades/tasks.py

@@ -3,7 +3,6 @@ This module contains tasks for asynchronous execution of grade updates.
 """
 
 from celery import task
-from celery.exceptions import Retry
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
@@ -56,6 +55,14 @@ class _BaseTask(PersistOnFailureTask, LoggedTask):  # pylint: disable=abstract-m
     abstract = True
 
 
+@task
+def compute_grades_for_course(course_key, offset, batch_size):  # pylint: disable=unused-argument
+    """
+    TODO: TNL-6690: Fill this task in and remove pylint disables
+    """
+    pass
+
+
 @task(bind=True, base=_BaseTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
 def recalculate_subsection_grade_v3(self, **kwargs):
     """

lms/djangoapps/instructor/management/commands/compute_grades.py

-#!/usr/bin/python
-"""
-django management command: dump grades to csv files
-for use by batch processes
-"""
-from django.http import Http404
-from django.core.management.base import BaseCommand
-
-from courseware.courses import get_course_by_id
-from lms.djangoapps.instructor.offline_gradecalc import offline_grade_calculation
-from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import CourseKey
-from opaque_keys.edx.locations import SlashSeparatedCourseKey
-
-
-class Command(BaseCommand):
-    help = "Compute grades for all students in a course, and store result in DB.\n"
-    help += "Usage: compute_grades course_id_or_dir \n"
-    help += "   course_id_or_dir: space separated list of either course_ids or course_dirs\n"
-    help += 'Example course_id: MITx/8.01rq_MW/Classical_Mechanics_Reading_Questions_Fall_2012_MW_Section'
-
-    def add_arguments(self, parser):
-        parser.add_argument('course_id_or_dir', nargs='+')
-
-    def handle(self, *args, **options):
-
-        print "options = ", options
-
-        try:
-            course_ids = options['course_id_or_dir']
-        except KeyError:
-            print self.help
-            return
-        course_key = None
-        # parse out the course id into a coursekey
-        for course_id in course_ids:
-            try:
-                course_key = CourseKey.from_string(course_id)
-            # if it's not a new-style course key, parse it from an old-style
-            # course key
-            except InvalidKeyError:
-                course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
-            try:
-                get_course_by_id(course_key)
-            except Http404 as err:
-                print "-----------------------------------------------------------------------------"
-                print "Sorry, cannot find course with id {}".format(course_id)
-                print "Got exception {}".format(err)
-                print "Please provide a course ID or course data directory name, eg content-mit-801rq"
-                return
-
-            print "-----------------------------------------------------------------------------"
-            print "Computing grades for {}".format(course_id)
-
-            offline_grade_calculation(course_key)

lms/djangoapps/instructor/management/tests/test_compute_grades.py

-# coding=utf-8
-
-"""Tests for Django instructor management commands"""
-
-from unittest import TestCase
-
-from django.core.management import call_command
-from mock import Mock
-
-from lms.djangoapps.instructor.offline_gradecalc import offline_grade_calculation  # pylint: disable=unused-import
-from opaque_keys.edx.keys import CourseKey
-from opaque_keys.edx.locator import CourseLocator
-
-
-class InstructorCommandsTest(TestCase):
-    """Unittest subclass for instructor module management commands."""
-
-    def test_compute_grades_command(self):
-        course_id = 'MITx/0.0001/2016_Fall'
-        offline_grade_calculation = Mock()  # pylint: disable=redefined-outer-name
-        CourseKey.from_string = Mock(return_value=CourseLocator(*course_id.split('/')))
-        call_command('compute_grades', )
-        self.asertEqual(offline_grade_calculation.call_count, 1)  # pylint: disable=no-member
-        offline_grade_calculation.assert_called_with(CourseKey.from_string('MITx/0.0001/2016_Fall'))
-
-    def test_compute_grades_command_multiple_courses(self):
-        course_id1 = 'MITx/0.0001/2016_Fall'
-        course_id2 = 'MITx/0.0002/2016_Fall'
-        CourseKey.from_string = Mock()
-        offline_grade_calculation = Mock()  # pylint: disable=redefined-outer-name
-        call_command('compute_grades', '{0} {1}'.format(course_id1, course_id1))
-        self.asertEqual(offline_grade_calculation.call_count, 2)  # pylint: disable=no-member
-        CourseKey.from_string.assert_called_with(course_id1)
-        CourseKey.from_string.assert_called_with(course_id2)

openedx/core/djangoapps/content/block_structure/management/commands/tests/test_generate_course_blocks.py

@@ -158,11 +158,11 @@ class TestGenerateCourseBlocks(ModuleStoreTestCase):
             self.command.handle(all_courses=False)
 
     def test_no_course_mode(self):
-        with self.assertRaisesMessage(CommandError, 'Either --courses or --all_courses must be specified.'):
+        with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --courses, --all_courses'):
             self.command.handle()
 
     def test_both_course_modes(self):
-        with self.assertRaisesMessage(CommandError, 'Both --courses and --all_courses cannot be specified.'):
+        with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --courses, --all_courses'):
             self.command.handle(all_courses=True, courses=['some/course/key'])
 
     @ddt.data(

openedx/core/lib/command_utils.py

@@ -8,17 +8,18 @@ from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 
 
-def get_mutually_exclusive_required_option(options, option_1, option_2):
+def get_mutually_exclusive_required_option(options, *selections):
     """
     Validates that exactly one of the 2 given options is specified.
     Returns the name of the found option.
     """
-    validate_mutually_exclusive_option(options, option_1, option_2)
 
-    if not options.get(option_1) and not options.get(option_2):
-        raise CommandError('Either --{} or --{} must be specified.'.format(option_1, option_2))
+    selected = [sel for sel in selections if options.get(sel)]
+    if len(selected) != 1:
+        selection_string = u', '.join('--{}'.format(selection) for selection in selections)
 
-    return option_1 if options.get(option_1) else option_2
+        raise CommandError(u'Must specify exactly one of {}'.format(selection_string))
+    return selected[0]
 
 
 def validate_mutually_exclusive_option(options, option_1, option_2):

openedx/core/lib/tests/test_command_utils.py

+"""
+Tests of management command utility code
+"""
+
+from unittest import TestCase
+
+import ddt
+from django.core.management import CommandError
+
+from .. import command_utils
+
+
+@ddt.ddt
+class MutuallyExclusiveRequiredOptionsTestCase(TestCase):
+    """
+    Test that mutually exclusive required options allow one and only one option
+    to be specified with a true value.
+    """
+    @ddt.data(
+        (['opta'], {'opta': 1}, 'opta'),
+        (['opta', 'optb'], {'opta': 1}, 'opta'),
+        (['opta', 'optb'], {'optb': 1}, 'optb'),
+        (['opta', 'optb'], {'opta': 1, 'optc': 1}, 'opta'),
+        (['opta', 'optb'], {'opta': 1, 'optb': 0}, 'opta'),
+        (['opta', 'optb', 'optc'], {'optc': 1, 'optd': 1}, 'optc'),
+        (['opta', 'optb', 'optc'], {'optc': 1}, 'optc'),
+        (['opta', 'optb', 'optc'], {'optd': 0, 'optc': 1}, 'optc'),
+    )
+    @ddt.unpack
+    def test_successful_exclusive_options(self, exclusions, opts, expected):
+        result = command_utils.get_mutually_exclusive_required_option(opts, *exclusions)
+        self.assertEqual(result, expected)
+
+    @ddt.data(
+        (['opta'], {'opta': 0}),
+        (['opta', 'optb'], {'opta': 1, 'optb': 1}),
+        (['opta', 'optb'], {'optc': 1, 'optd': 1}),
+        (['opta', 'optb'], {}),
+        (['opta', 'optb', 'optc'], {'opta': 1, 'optc': 1}),
+        (['opta', 'optb', 'optc'], {'opta': 1, 'optb': 1}),
+        (['opta', 'optb', 'optc'], {'optb': 1, 'optc': 1}),
+        (['opta', 'optb', 'optc'], {'opta': 1, 'optb': 1, 'optc': 1}),
+        (['opta', 'optb', 'optc'], {}),
+    )
+    @ddt.unpack
+    def test_invalid_exclusive_options(self, exclusions, opts):
+        with self.assertRaises(CommandError):
+            command_utils.get_mutually_exclusive_required_option(opts, *exclusions)