From a3154b923f65d33203a83d406b0bebfd1ee99a74 Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Wed, 22 Mar 2017 14:19:48 -0400 Subject: [PATCH] Create task to process a batch of grade computations. TNL-6690 --- lms/djangoapps/grades/new/course_grade.py | 4 +- lms/djangoapps/grades/tasks.py | 35 ++++++---- lms/djangoapps/grades/tests/test_tasks.py | 80 +++++++++++++++++++---- 3 files changed, 90 insertions(+), 29 deletions(-) diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index c80a8a656bd0..03531116dbd4 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -368,7 +368,7 @@ def create(self, student, course, collected_block_structure=None, read_only=True GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'err_msg']) - def iter(self, course, students): + def iter(self, course, students, read_only=True): """ Given a course and an iterable of students (User), yield a GradeResult for every student enrolled in the course. GradeResult is a named tuple of: @@ -388,7 +388,7 @@ def iter(self, course, students): for student in students: with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=[u'action:{}'.format(course.id)]): try: - course_grade = CourseGradeFactory().create(student, course, collected_block_structure) + course_grade = CourseGradeFactory().create(student, course, collected_block_structure, read_only=read_only) yield self.GradeResult(student, course_grade, "") except Exception as exc: # pylint: disable=broad-except diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index e43a9bf75185..e3acd10acbd5 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -19,8 +19,10 @@ from celery_utils.persist_on_failure import PersistOnFailureTask from courseware.model_data import get_score from lms.djangoapps.course_blocks.api import get_course_blocks -from opaque_keys.edx.keys import UsageKey +from lms.djangoapps.courseware import courses +from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import CourseLocator +from student.models import CourseEnrollment from submissions import api as sub_api from track.event_transaction_utils import ( set_event_transaction_type, @@ -31,6 +33,7 @@ from .constants import ScoreDatabaseTableEnum from .new.subsection_grade import SubsectionGradeFactory +from .new.course_grade import CourseGradeFactory from .signals.signals import SUBSECTION_SCORE_CHANGED from .transformer import GradesTransformer @@ -58,12 +61,24 @@ 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 +@task(base=_BaseTask) +def compute_grades_for_course(course_key, offset, batch_size): """ - TODO: TNL-6690: Fill this task in and remove pylint disables + Compute grades for a set of students in the specified course. + + The set of students will be determined by the order of enrollment date, and + limited to at most students, starting from the specified + offset. """ - pass + + course = courses.get_course_by_id(CourseKey.from_string(course_key)) + enrollments = CourseEnrollment.objects.filter(course_id=course.id).order_by('created') + student_iter = (enrollment.user for enrollment in enrollments[offset:offset + batch_size]) + list(CourseGradeFactory().iter( + course, + students=student_iter, + read_only=False, + )) @task(bind=True, base=_BaseTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) @@ -136,7 +151,7 @@ def _recalculate_subsection_grade(self, **kwargs): self.request.id, kwargs, )) - raise _retry_recalculate_subsection_grade(self, exc=exc, **kwargs) + raise self.retry(kwargs=kwargs, exc=exc) def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs): @@ -218,11 +233,3 @@ def _update_subsection_grades( user=student, subsection_grade=subsection_grade, ) - - -def _retry_recalculate_subsection_grade(self, exc=None, **kwargs): - """ - Calls retry for the recalculate_subsection_grade task with the - given inputs. - """ - self.retry(kwargs=kwargs, exc=exc) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 33fdd277080d..d6aa9e221ea6 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -11,10 +11,11 @@ import itertools from mock import patch, MagicMock import pytz +import six from util.date_utils import to_timestamp from openedx.core.djangoapps.content.block_structure.exceptions import BlockStructureNotFound -from student.models import anonymous_id_for_user +from student.models import CourseEnrollment, anonymous_id_for_user from student.tests.factories import UserFactory from track.event_transaction_utils import ( create_new_event_transaction_id, @@ -29,22 +30,17 @@ from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED -from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3, RECALCULATE_GRADE_DELAY +from lms.djangoapps.grades.tasks import ( + compute_grades_for_course, + recalculate_subsection_grade_v3, + RECALCULATE_GRADE_DELAY +) -@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) -@ddt.ddt -class RecalculateSubsectionGradeTest(ModuleStoreTestCase): +class HasCourseWithProblemsMixin(object): """ - Ensures that the recalculate subsection grade task functions as expected when run. + Mixin to provide tests with a sample course with graded subsections """ - ENABLED_SIGNALS = ['course_published', 'pre_publish'] - - def setUp(self): - super(RecalculateSubsectionGradeTest, self).setUp() - self.user = UserFactory() - PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True) - def set_up_course(self, enable_persistent_grades=True, create_multiple_subsections=False): """ Configures the course for this test. @@ -100,6 +96,20 @@ def set_up_course(self, enable_persistent_grades=True, create_multiple_subsectio _ = anonymous_id_for_user(self.user, self.course.id) # pylint: enable=attribute-defined-outside-init,no-member + +@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) +@ddt.ddt +class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTestCase): + """ + Ensures that the recalculate subsection grade task functions as expected when run. + """ + ENABLED_SIGNALS = ['course_published', 'pre_publish'] + + def setUp(self): + super(RecalculateSubsectionGradeTest, self).setUp() + self.user = UserFactory() + PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True) + @contextmanager def mock_get_score(self, score=MagicMock(grade=1.0, max_grade=2.0)): """ @@ -363,3 +373,47 @@ def _assert_retry_not_called(self, mock_retry): Verifies the task was not retried. """ self.assertFalse(mock_retry.called) + + +@ddt.ddt +class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase): + """ + Test compute_grades_for_course task. + """ + + ENABLED_SIGNALS = ['course_published', 'pre_publish'] + + def setUp(self): + super(ComputeGradesForCourseTest, self).setUp() + self.users = [UserFactory.create() for _ in xrange(12)] + self.set_up_course() + for user in self.users: + CourseEnrollment.enroll(user, self.course.id) + + @ddt.data(*xrange(0, 12, 3)) + def test_behavior(self, batch_size): + result = compute_grades_for_course.delay( + course_key=six.text_type(self.course.id), + batch_size=batch_size, + offset=4 + ) + self.assertTrue(result.successful) + self.assertEqual( + PersistentCourseGrade.objects.filter(course_id=self.course.id).count(), + min(batch_size, 8) # No more than 8 due to offset + ) + self.assertEqual( + PersistentSubsectionGrade.objects.filter(course_id=self.course.id).count(), + min(batch_size, 8) # No more than 8 due to offset + ) + + @ddt.data(*xrange(1, 12, 3)) + def test_database_calls(self, batch_size): + per_user_queries = 16 * min(batch_size, 6) # No more than 6 due to offset + with self.assertNumQueries(3 + 16 * min(batch_size, 6)): + with check_mongo_calls(1): + compute_grades_for_course.delay( + course_key=six.text_type(self.course.id), + batch_size=batch_size, + offset=6, + )