From bdc0b33cb423f612eeb13909f7458a8a10889c5e Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Tue, 28 Mar 2017 15:16:27 -0400 Subject: [PATCH] add settings for compute grades command --- lms/djangoapps/grades/admin.py | 7 ++++- lms/djangoapps/grades/config/models.py | 16 ++++++++++- .../management/commands/compute_grades.py | 22 ++++++++++++--- .../commands/tests/test_compute_grades.py | 13 +++++++++ .../migrations/0012_computegradessetting.py | 28 +++++++++++++++++++ 5 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 lms/djangoapps/grades/migrations/0012_computegradessetting.py diff --git a/lms/djangoapps/grades/admin.py b/lms/djangoapps/grades/admin.py index 437d3ffe04..aaecba7295 100644 --- a/lms/djangoapps/grades/admin.py +++ b/lms/djangoapps/grades/admin.py @@ -5,7 +5,11 @@ from django.contrib import admin from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin -from lms.djangoapps.grades.config.models import CoursePersistentGradesFlag, PersistentGradesEnabledFlag +from lms.djangoapps.grades.config.models import ( + CoursePersistentGradesFlag, + PersistentGradesEnabledFlag, + ComputeGradesSetting, +) from lms.djangoapps.grades.config.forms import CoursePersistentGradesAdminForm @@ -25,3 +29,4 @@ class CoursePersistentGradesAdmin(KeyedConfigurationModelAdmin): admin.site.register(CoursePersistentGradesFlag, CoursePersistentGradesAdmin) admin.site.register(PersistentGradesEnabledFlag, ConfigurationModelAdmin) +admin.site.register(ComputeGradesSetting, ConfigurationModelAdmin) diff --git a/lms/djangoapps/grades/config/models.py b/lms/djangoapps/grades/config/models.py index 5d1b15fe44..e0e745ca76 100644 --- a/lms/djangoapps/grades/config/models.py +++ b/lms/djangoapps/grades/config/models.py @@ -4,7 +4,7 @@ controlling persistent grades. """ from config_models.models import ConfigurationModel from django.conf import settings -from django.db.models import BooleanField +from django.db.models import BooleanField, IntegerField, TextField from openedx.core.djangoapps.xmodule_django.models import CourseKeyField from request_cache.middleware import request_cached @@ -71,3 +71,17 @@ class CoursePersistentGradesFlag(ConfigurationModel): not_en = "" # pylint: disable=no-member return u"Course '{}': Persistent Grades {}Enabled".format(self.course_id.to_deprecated_string(), not_en) + + +class ComputeGradesSetting(ConfigurationModel): + """ + ... + """ + class Meta(object): + app_label = "grades" + + batch_size = IntegerField(default=100) + course_ids = TextField( + blank=False, + help_text="Whitespace-separated list of course keys for which to compute grades." + ) diff --git a/lms/djangoapps/grades/management/commands/compute_grades.py b/lms/djangoapps/grades/management/commands/compute_grades.py index 4d19c7483f..b6d4ab0fcc 100644 --- a/lms/djangoapps/grades/management/commands/compute_grades.py +++ b/lms/djangoapps/grades/management/commands/compute_grades.py @@ -6,13 +6,14 @@ from __future__ import absolute_import, division, print_function, unicode_litera import logging -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError import six from openedx.core.lib.command_utils import ( get_mutually_exclusive_required_option, parse_course_keys, ) +from lms.djangoapps.grades.config.models import ComputeGradesSetting from student.models import CourseEnrollment from xmodule.modulestore.django import modulestore @@ -47,6 +48,12 @@ class Command(BaseCommand): action='store_true', default=False, ) + parser.add_argument( + '--from_settings', + help='Compute grades with settings set via Django admin', + action='store_true', + default=False, + ) parser.add_argument( '--routing_key', dest='routing_key', @@ -84,10 +91,11 @@ class Command(BaseCommand): # 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 {} + batch_size = self._latest_settings().batch_size if options.get('from_settings') else options['batch_size'] kwargs = { 'course_key': six.text_type(course_key), 'offset': offset, - 'batch_size': options['batch_size'], + 'batch_size': batch_size, } result = tasks.compute_grades_for_course.apply_async( kwargs=kwargs, @@ -103,11 +111,14 @@ class Command(BaseCommand): """ Return a list of courses that need scores computed. """ - courses_mode = get_mutually_exclusive_required_option(options, 'courses', 'all_courses') + + courses_mode = get_mutually_exclusive_required_option(options, 'courses', 'all_courses', 'from_settings') if courses_mode == 'all_courses': course_keys = [course.id for course in modulestore().get_course_summaries()] - else: + elif courses_mode == 'courses': course_keys = parse_course_keys(options['courses']) + else: + course_keys = parse_course_keys(self._latest_settings().course_ids.split()) return course_keys def _set_log_level(self, options): @@ -120,3 +131,6 @@ class Command(BaseCommand): elif options.get('verbosity') >= 1: log_level = logging.INFO log.setLevel(log_level) + + def _latest_settings(self): + return ComputeGradesSetting.objects.order_by('-id')[0] diff --git a/lms/djangoapps/grades/management/commands/tests/test_compute_grades.py b/lms/djangoapps/grades/management/commands/tests/test_compute_grades.py index 319030f466..9a063188d5 100644 --- a/lms/djangoapps/grades/management/commands/tests/test_compute_grades.py +++ b/lms/djangoapps/grades/management/commands/tests/test_compute_grades.py @@ -16,6 +16,7 @@ from student.models import CourseEnrollment from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from lms.djangoapps.grades.config.models import ComputeGradesSetting from lms.djangoapps.grades.management.commands import compute_grades @@ -59,6 +60,18 @@ class TestComputeGrades(SharedModuleStoreTestCase): with self.assertRaises(CommandError): self.command._get_course_keys({'courses': [self.course_keys[0], self.course_keys[1], 'badcoursekey']}) + def test_from_settings(self): + ComputeGradesSetting.objects.create(course_ids=" ".join(self.course_keys)) + courses = self.command._get_course_keys({'from_settings': True}) + self.assertEqual( + sorted(six.text_type(course) for course in courses), + self.course_keys, + ) + # test that --from_settings always uses the latest setting + ComputeGradesSetting.objects.create(course_ids='badcoursekey') + with self.assertRaises(CommandError): + self.command._get_course_keys({'from_settings': True}) + @patch('lms.djangoapps.grades.tasks.compute_grades_for_course') def test_tasks_fired(self, mock_task): call_command( diff --git a/lms/djangoapps/grades/migrations/0012_computegradessetting.py b/lms/djangoapps/grades/migrations/0012_computegradessetting.py new file mode 100644 index 0000000000..143c191ee1 --- /dev/null +++ b/lms/djangoapps/grades/migrations/0012_computegradessetting.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +from django.conf import settings + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('grades', '0011_null_edited_time'), + ] + + operations = [ + migrations.CreateModel( + name='ComputeGradesSetting', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('batch_size', models.IntegerField(default=100)), + ('course_ids', models.TextField(help_text=b'Whitespace-separated list of course keys for which to compute grades.')), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + ), + ]