Commit 379a2c18 by Nimisha Asthagiri

Refactor Grade Report in prep for parallelization.

parent cc98111d
......@@ -564,7 +564,7 @@ def ccx_grades_csv(request, course, ccx=None):
courseenrollment__course_id=ccx_key,
courseenrollment__is_active=1
).order_by('username').select_related("profile")
grades = CourseGradeFactory().iter(course, enrolled_students)
grades = CourseGradeFactory().iter(enrolled_students, course)
header = None
rows = []
......
from lms.djangoapps.course_blocks.api import get_course_blocks
from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager
from xmodule.modulestore.django import modulestore
from ..transformer import GradesTransformer
......@@ -57,6 +58,12 @@ class CourseData(object):
return self._structure
@property
def collected_structure(self):
if not self._collected_block_structure:
self._collected_block_structure = get_block_structure_manager(self.course_key).get_collected()
return self._collected_block_structure
@property
def course(self):
if not self._course:
self._course = modulestore().get_course(self.course_key)
......
......@@ -2,7 +2,6 @@ from collections import namedtuple
import dogstats_wrapper as dog_stats_api
from logging import getLogger
from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager
from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED
from ..config import assume_zero_if_absent, should_persist_grades
......@@ -77,7 +76,15 @@ class CourseGradeFactory(object):
course_data = CourseData(user, course, collected_block_structure, course_structure, course_key)
return self._update(user, course_data, read_only=False)
def iter(self, course, students, force_update=False):
def iter(
self,
users,
course=None,
collected_block_structure=None,
course_structure=None,
course_key=None,
force_update=False,
):
"""
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:
......@@ -92,25 +99,27 @@ class CourseGradeFactory(object):
# compute the grade for all students.
# 2. Optimization: the collected course_structure is not
# retrieved from the data store multiple times.
collected_block_structure = get_block_structure_manager(course.id).get_collected()
for student in students:
with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=[u'action:{}'.format(course.id)]):
course_data = CourseData(None, course, collected_block_structure, course_structure, course_key)
for user in users:
with dog_stats_api.timer(
'lms.grades.CourseGradeFactory.iter',
tags=[u'action:{}'.format(course_data.course_key)]
):
try:
operation = CourseGradeFactory().update if force_update else CourseGradeFactory().create
course_grade = operation(student, course, collected_block_structure)
yield self.GradeResult(student, course_grade, "")
method = CourseGradeFactory().update if force_update else CourseGradeFactory().create
course_grade = method(user, course, course_data.collected_structure, course_structure, course_key)
yield self.GradeResult(user, course_grade, "")
except Exception as exc: # pylint: disable=broad-except
# Keep marching on even if this student couldn't be graded for
# some reason, but log it for future reference.
log.exception(
'Cannot grade student %s in course %s because of exception: %s',
student.id,
course.id,
user.id,
course_data.course_key,
exc.message
)
yield self.GradeResult(student, None, exc.message)
yield self.GradeResult(user, None, exc.message)
@staticmethod
def _create_zero(user, course_data):
......
......@@ -96,7 +96,7 @@ def compute_grades_for_course(course_key, offset, batch_size, **kwargs): # pyli
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, force_update=True))
list(CourseGradeFactory().iter(users=student_iter, course=course, force_update=True))
@task(bind=True, base=_BaseTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
......
......@@ -60,7 +60,7 @@ class TestGradeIteration(SharedModuleStoreTestCase):
If we don't pass in any students, it should return a zero-length
iterator, but it shouldn't error.
"""
grade_results = list(CourseGradeFactory().iter(self.course, []))
grade_results = list(CourseGradeFactory().iter([], self.course))
self.assertEqual(grade_results, [])
def test_all_empty_grades(self):
......@@ -130,7 +130,7 @@ class TestGradeIteration(SharedModuleStoreTestCase):
students_to_course_grades = {}
students_to_errors = {}
for student, course_grade, err_msg in CourseGradeFactory().iter(course, students):
for student, course_grade, err_msg in CourseGradeFactory().iter(students, course):
students_to_course_grades[student] = course_grade
if err_msg:
students_to_errors[student] = err_msg
......
......@@ -39,9 +39,9 @@ from lms.djangoapps.instructor_task.tasks_helper.enrollments import (
upload_students_csv,
)
from lms.djangoapps.instructor_task.tasks_helper.grades import (
generate_course_grade_report,
generate_problem_grade_report,
upload_problem_responses_csv,
CourseGradeReport,
ProblemGradeReport,
ProblemResponses,
)
from lms.djangoapps.instructor_task.tasks_helper.misc import (
cohort_students_and_upload,
......@@ -160,7 +160,7 @@ def calculate_problem_responses_csv(entry_id, xmodule_instance_args):
"""
# Translators: This is a past-tense verb that is inserted into task progress messages as {action}.
action_name = ugettext_noop('generated')
task_fn = partial(upload_problem_responses_csv, xmodule_instance_args)
task_fn = partial(ProblemResponses.generate, xmodule_instance_args)
return run_main_task(entry_id, task_fn, action_name)
......@@ -176,7 +176,7 @@ def calculate_grades_csv(entry_id, xmodule_instance_args):
xmodule_instance_args.get('task_id'), entry_id, action_name
)
task_fn = partial(generate_course_grade_report, xmodule_instance_args)
task_fn = partial(CourseGradeReport.generate, xmodule_instance_args)
return run_main_task(entry_id, task_fn, action_name)
......@@ -193,7 +193,7 @@ def calculate_problem_grade_report(entry_id, xmodule_instance_args):
xmodule_instance_args.get('task_id'), entry_id, action_name
)
task_fn = partial(generate_problem_grade_report, xmodule_instance_args)
task_fn = partial(ProblemGradeReport.generate, xmodule_instance_args)
return run_main_task(entry_id, task_fn, action_name)
......
......@@ -32,7 +32,7 @@ from lms.djangoapps.instructor_task.api import (
submit_delete_problem_state_for_all_students
)
from lms.djangoapps.instructor_task.models import InstructorTask
from lms.djangoapps.instructor_task.tasks_helper.grades import generate_course_grade_report
from lms.djangoapps.instructor_task.tasks_helper.grades import CourseGradeReport
from lms.djangoapps.instructor_task.tests.test_base import (
InstructorTaskModuleTestCase,
TestReportMixin,
......@@ -572,10 +572,10 @@ class TestGradeReportConditionalContent(TestReportMixin, TestConditionalContent,
def verify_csv_task_success(self, task_result):
"""
Verify that all students were successfully graded by
`generate_course_grade_report`.
`CourseGradeReport`.
Arguments:
task_result (dict): Return value of `generate_course_grade_report`.
task_result (dict): Return value of `CourseGradeReport.generate`.
"""
self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, task_result)
......@@ -636,7 +636,7 @@ class TestGradeReportConditionalContent(TestReportMixin, TestConditionalContent,
self.submit_student_answer(self.student_b.username, problem_b_url, [OPTION_1, OPTION_2])
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = generate_course_grade_report(None, None, self.course.id, None, 'graded')
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
self.verify_csv_task_success(result)
self.verify_grades_in_csv(
[
......@@ -669,7 +669,7 @@ class TestGradeReportConditionalContent(TestReportMixin, TestConditionalContent,
self.submit_student_answer(self.student_a.username, problem_a_url, [OPTION_1, OPTION_1])
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = generate_course_grade_report(None, None, self.course.id, None, 'graded')
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
self.verify_csv_task_success(result)
self.verify_grades_in_csv(
[
......
......@@ -59,14 +59,13 @@ from lms.djangoapps.instructor_task.tasks_helper.enrollments import (
upload_students_csv,
)
from lms.djangoapps.instructor_task.tasks_helper.grades import (
generate_course_grade_report,
generate_problem_grade_report,
upload_problem_responses_csv,
CourseGradeReport,
ProblemGradeReport,
ProblemResponses,
)
from lms.djangoapps.instructor_task.tasks_helper.misc import (
cohort_students_and_upload,
upload_course_survey_report,
upload_proctored_exam_results_report,
upload_ora2_data,
)
from ..tasks_helper.utils import (
......@@ -89,7 +88,7 @@ class InstructorGradeReportTestCase(TestReportMixin, InstructorTaskCourseTestCas
Verify cell data in the grades CSV for a particular user.
"""
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = generate_course_grade_report(None, None, course_id, None, 'graded')
result = CourseGradeReport.generate(None, None, course_id, None, 'graded')
self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, result)
report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
report_csv_filename = report_store.links_for(course_id)[0][0]
......@@ -121,7 +120,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
self.current_task.update_state = Mock()
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') as mock_current_task:
mock_current_task.return_value = self.current_task
result = generate_course_grade_report(None, None, self.course.id, None, 'graded')
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
num_students = len(emails)
self.assertDictContainsSubset({'attempted': num_students, 'succeeded': num_students, 'failed': 0}, result)
......@@ -135,7 +134,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
mock_grades_iter.return_value = [
(self.create_student('username', 'student@example.com'), None, 'Cannot grade student')
]
result = generate_course_grade_report(None, None, self.course.id, None, 'graded')
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset({'attempted': 1, 'succeeded': 0, 'failed': 1}, result)
report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
......@@ -319,7 +318,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
'',
)
]
result = generate_course_grade_report(None, None, self.course.id, None, 'graded')
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset({'attempted': 1, 'succeeded': 1, 'failed': 0}, result)
......@@ -378,7 +377,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskCourseTestCase):
{'username': 'user1', 'state': u'state1'},
{'username': 'user2', 'state': u'state2'},
]
result = upload_problem_responses_csv(None, None, self.course.id, task_input, 'calculated')
result = ProblemResponses.generate(None, None, self.course.id, task_input, 'calculated')
report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
links = report_store.links_for(self.course.id)
......@@ -609,7 +608,7 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
Verify that we see no grade information for a course with no graded
problems.
"""
result = generate_problem_grade_report(None, None, self.course.id, None, 'graded')
result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset({'action_name': 'graded', 'attempted': 2, 'succeeded': 2, 'failed': 0}, result)
self.verify_rows_in_csv([
dict(zip(
......@@ -633,7 +632,7 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
self.define_option_problem(u'Problem1', parent=vertical)
self.submit_student_answer(self.student_1.username, u'Problem1', ['Option 1'])
result = generate_problem_grade_report(None, None, self.course.id, None, 'graded')
result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset({'action_name': 'graded', 'attempted': 2, 'succeeded': 2, 'failed': 0}, result)
problem_name = u'Homework 1: Subsection - Problem1'
header_row = self.csv_header_row + [problem_name + ' (Earned)', problem_name + ' (Possible)']
......@@ -670,7 +669,7 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
mock_grades_iter.return_value = [
(student, None, error_message)
]
result = generate_problem_grade_report(None, None, self.course.id, None, 'graded')
result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset({'attempted': 1, 'succeeded': 0, 'failed': 1}, result)
report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
......@@ -720,7 +719,7 @@ class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent,
self.submit_student_answer(self.student_b.username, self.problem_b_url, [self.OPTION_1, self.OPTION_2])
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = generate_problem_grade_report(None, None, self.course.id, None, 'graded')
result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset(
{'action_name': 'graded', 'attempted': 2, 'succeeded': 2, 'failed': 0}, result
)
......@@ -812,7 +811,7 @@ class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent,
header_row += [problem + ' (Earned)', problem + ' (Possible)']
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
generate_problem_grade_report(None, None, self.course.id, None, 'graded')
ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertEquals(self.get_csv_row_with_headers(), header_row)
......@@ -868,7 +867,7 @@ class TestProblemReportCohortedContent(TestReportMixin, ContentGroupTestCase, In
self.submit_student_answer(self.beta_user.username, u'Problem1', ['Option 1', 'Option 2'])
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = generate_problem_grade_report(None, None, self.course.id, None, 'graded')
result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset(
{'action_name': 'graded', 'attempted': 4, 'succeeded': 4, 'failed': 0}, result
)
......@@ -1579,7 +1578,7 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = generate_course_grade_report(None, None, self.course.id, None, 'graded')
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset(
{'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0},
result,
......@@ -1654,7 +1653,7 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
Verify grade report data.
"""
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
generate_course_grade_report(None, None, self.course.id, None, 'graded')
CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
report_csv_filename = report_store.links_for(self.course.id)[0][0]
report_path = report_store.path_to(self.course.id, report_csv_filename)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment