Bulk-reads and Request caching in Course Grade Report

This reverts commit 5388d5d1fc.
This commit is contained in:
Nimisha Asthagiri
2017-05-10 16:05:32 -04:00
parent 544d5d5996
commit adb88e21f3
22 changed files with 477 additions and 130 deletions

View File

@@ -493,6 +493,18 @@ def handle_course_cert_awarded(sender, user, course_key, **kwargs): # pylint: d
def certificate_status_for_student(student, course_id):
"""
This returns a dictionary with a key for status, and other information.
See certificate_status for more information.
"""
try:
generated_certificate = GeneratedCertificate.objects.get(user=student, course_id=course_id)
except GeneratedCertificate.DoesNotExist:
generated_certificate = None
return certificate_status(generated_certificate)
def certificate_status(generated_certificate):
'''
This returns a dictionary with a key for status, and other information.
The status is one of the following:
@@ -527,9 +539,7 @@ def certificate_status_for_student(student, course_id):
# the course_modes app is loaded, resulting in a Django deprecation warning.
from course_modes.models import CourseMode
try:
generated_certificate = GeneratedCertificate.objects.get( # pylint: disable=no-member
user=student, course_id=course_id)
if generated_certificate:
cert_status = {
'status': generated_certificate.status,
'mode': generated_certificate.mode,
@@ -539,7 +549,7 @@ def certificate_status_for_student(student, course_id):
cert_status['grade'] = generated_certificate.grade
if generated_certificate.mode == 'audit':
course_mode_slugs = [mode.slug for mode in CourseMode.modes_for_course(course_id)]
course_mode_slugs = [mode.slug for mode in CourseMode.modes_for_course(generated_certificate.course_id)]
# Short term fix to make sure old audit users with certs still see their certs
# only do this if there if no honor mode
if 'honor' not in course_mode_slugs:
@@ -550,31 +560,24 @@ def certificate_status_for_student(student, course_id):
cert_status['download_url'] = generated_certificate.download_url
return cert_status
except GeneratedCertificate.DoesNotExist:
pass
return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None}
else:
return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None}
def certificate_info_for_user(user, course_id, grade, user_is_whitelisted=None):
def certificate_info_for_user(user, grade, user_is_whitelisted, user_certificate):
"""
Returns the certificate info for a user for grade report.
"""
if user_is_whitelisted is None:
user_is_whitelisted = CertificateWhitelist.objects.filter(
user=user, course_id=course_id, whitelist=True
).exists()
certificate_is_delivered = 'N'
certificate_type = 'N/A'
eligible_for_certificate = 'Y' if (user_is_whitelisted or grade is not None) and user.profile.allow_certificate \
else 'N'
certificate_status = certificate_status_for_student(user, course_id)
certificate_generated = certificate_status['status'] == CertificateStatuses.downloadable
status = certificate_status(user_certificate)
certificate_generated = status['status'] == CertificateStatuses.downloadable
if certificate_generated:
certificate_is_delivered = 'Y'
certificate_type = certificate_status['mode']
certificate_type = status['mode']
return [eligible_for_certificate, certificate_is_delivered, certificate_type]

View File

@@ -54,11 +54,11 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
Verify that certificate_info_for_user works.
"""
student = UserFactory()
course = CourseFactory.create(org='edx', number='verified', display_name='Verified Course')
_ = CourseFactory.create(org='edx', number='verified', display_name='Verified Course')
student.profile.allow_certificate = allow_certificate
student.profile.save()
certificate_info = certificate_info_for_user(student, course.id, grade, whitelisted)
certificate_info = certificate_info_for_user(student, grade, whitelisted, user_certificate=None)
self.assertEqual(certificate_info, output)
@unpack
@@ -81,14 +81,13 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
student.profile.allow_certificate = allow_certificate
student.profile.save()
GeneratedCertificateFactory.create(
certificate = GeneratedCertificateFactory.create(
user=student,
course_id=course.id,
status=CertificateStatuses.downloadable,
mode='honor'
)
certificate_info = certificate_info_for_user(student, course.id, grade, whitelisted)
certificate_info = certificate_info_for_user(student, grade, whitelisted, certificate)
self.assertEqual(certificate_info, output)
def test_course_ids_with_certs_for_user(self):

View File

@@ -25,6 +25,7 @@ from track.event_transaction_utils import get_event_transaction_id, get_event_tr
from coursewarehistoryextended.fields import UnsignedBigIntAutoField
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx.core.djangoapps.xmodule_django.models import CourseKeyField, UsageKeyField
from request_cache import get_cache
from .config import waffle
@@ -522,6 +523,8 @@ class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel):
# Information related to course completion
passed_timestamp = models.DateTimeField(u'Date learner earned a passing grade', blank=True, null=True)
CACHE_NAMESPACE = u"grades.models.PersistentCourseGrade"
def __unicode__(self):
"""
Returns a string representation of this model.
@@ -535,6 +538,21 @@ class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel):
u"passed timestamp: {}".format(self.passed_timestamp),
])
@classmethod
def _cache_key(cls, course_id):
return u"grades_cache.{}".format(course_id)
@classmethod
def prefetch(cls, course_id, users):
"""
Prefetches grades for the given users for the given course.
"""
get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_id)] = {
grade.user_id: grade
for grade in
cls.objects.filter(user_id__in=[user.id for user in users], course_id=course_id)
}
@classmethod
def read(cls, user_id, course_id):
"""
@@ -546,7 +564,17 @@ class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel):
Raises PersistentCourseGrade.DoesNotExist if applicable
"""
return cls.objects.get(user_id=user_id, course_id=course_id)
try:
prefetched_grades = get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_id)]
try:
return prefetched_grades[user_id]
except KeyError:
# user's grade is not in the prefetched list, so
# assume they have no grade
raise cls.DoesNotExist
except KeyError:
# grades were not prefetched for the course, so fetch it
return cls.objects.get(user_id=user_id, course_id=course_id)
@classmethod
def update_or_create(cls, user_id, course_id, **kwargs):

View File

@@ -81,7 +81,6 @@ class CourseGradeFactory(object):
users,
course=None,
collected_block_structure=None,
course_structure=None,
course_key=None,
force_update=False,
):
@@ -99,7 +98,9 @@ class CourseGradeFactory(object):
# compute the grade for all students.
# 2. Optimization: the collected course_structure is not
# retrieved from the data store multiple times.
course_data = CourseData(None, course, collected_block_structure, course_structure, course_key)
course_data = CourseData(
user=None, course=course, collected_block_structure=collected_block_structure, course_key=course_key,
)
for user in users:
with dog_stats_api.timer(
'lms.grades.CourseGradeFactory.iter',
@@ -107,7 +108,9 @@ class CourseGradeFactory(object):
):
try:
method = CourseGradeFactory().update if force_update else CourseGradeFactory().create
course_grade = method(user, course, course_data.collected_structure, course_structure, course_key)
course_grade = method(
user, course_data.course, course_data.collected_structure, course_key=course_key,
)
yield self.GradeResult(user, course_grade, None)
except Exception as exc: # pylint: disable=broad-except

View File

@@ -178,10 +178,10 @@ class TestCourseGradeFactory(GradeTestBase):
self.assertEqual(course_grade.letter_grade, u'Pass' if expected_pass else None)
self.assertEqual(course_grade.percent, 0.5)
with self.assertNumQueries(12), mock_get_score(1, 2):
with self.assertNumQueries(11), mock_get_score(1, 2):
_assert_create(expected_pass=True)
with self.assertNumQueries(15), mock_get_score(1, 2):
with self.assertNumQueries(13), mock_get_score(1, 2):
grade_factory.update(self.request.user, self.course)
with self.assertNumQueries(1):
@@ -189,7 +189,7 @@ class TestCourseGradeFactory(GradeTestBase):
self._update_grading_policy(passing=0.9)
with self.assertNumQueries(8):
with self.assertNumQueries(6):
_assert_create(expected_pass=False)
@ddt.data(True, False)

View File

@@ -409,8 +409,8 @@ class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase
@ddt.data(*xrange(1, 12, 3))
def test_database_calls(self, batch_size):
per_user_queries = 17 * min(batch_size, 6) # No more than 6 due to offset
with self.assertNumQueries(5 + per_user_queries):
per_user_queries = 15 * min(batch_size, 6) # No more than 6 due to offset
with self.assertNumQueries(6 + per_user_queries):
with check_mongo_calls(1):
compute_grades_for_course_v2.delay(
course_key=six.text_type(self.course.id),

View File

@@ -12,14 +12,19 @@ from time import time
from instructor_analytics.basic import list_problem_responses
from instructor_analytics.csvs import format_dictlist
from certificates.models import CertificateWhitelist, certificate_info_for_user
from certificates.models import CertificateWhitelist, certificate_info_for_user, GeneratedCertificate
from courseware.courses import get_course_by_id
from lms.djangoapps.grades.context import grading_context_for_course
from lms.djangoapps.grades.context import grading_context_for_course, grading_context
from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory
from lms.djangoapps.grades.models import PersistentCourseGrade
from lms.djangoapps.teams.models import CourseTeamMembership
from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
from openedx.core.djangoapps.course_groups.cohorts import get_cohort, is_course_cohorted
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
from openedx.core.djangoapps.course_groups.cohorts import get_cohort, is_course_cohorted, bulk_cache_cohorts
from openedx.core.djangoapps.user_api.course_tag.api import BulkCourseTags
from student.models import CourseEnrollment
from student.roles import BulkRoleCache
from xmodule.modulestore.django import modulestore
from xmodule.partitions.partitions_service import PartitionService
from xmodule.split_test_module import get_split_user_partitions
@@ -30,7 +35,7 @@ from .utils import upload_csv_to_report_store
TASK_LOG = logging.getLogger('edx.celery.task')
class CourseGradeReportContext(object):
class _CourseGradeReportContext(object):
"""
Internal class that provides a common context to use for a single grade
report. When a report is parallelized across multiple processes,
@@ -57,6 +62,10 @@ class CourseGradeReportContext(object):
def course(self):
return get_course_by_id(self.course_id)
@lazy
def course_structure(self):
return get_course_in_cache(self.course_id)
@lazy
def course_experiments(self):
return get_split_user_partitions(self.course.user_partitions)
@@ -75,9 +84,9 @@ class CourseGradeReportContext(object):
Returns an OrderedDict that maps an assignment type to a dict of
subsection-headers and average-header.
"""
grading_context = grading_context_for_course(self.course_id)
grading_cxt = grading_context(self.course_structure)
graded_assignments_map = OrderedDict()
for assignment_type_name, subsection_infos in grading_context['all_graded_subsections_by_type'].iteritems():
for assignment_type_name, subsection_infos in grading_cxt['all_graded_subsections_by_type'].iteritems():
graded_subsections_map = OrderedDict()
for subsection_index, subsection_info in enumerate(subsection_infos, start=1):
subsection = subsection_info['subsection_block']
@@ -112,17 +121,64 @@ class CourseGradeReportContext(object):
return self.task_progress.update_task_state(extra_meta={'step': message})
class _CertificateBulkContext(object):
def __init__(self, context, users):
certificate_whitelist = CertificateWhitelist.objects.filter(course_id=context.course_id, whitelist=True)
self.whitelisted_user_ids = [entry.user_id for entry in certificate_whitelist]
self.certificates_by_user = {
certificate.user.id: certificate
for certificate in
GeneratedCertificate.objects.filter(course_id=context.course_id, user__in=users)
}
class _TeamBulkContext(object):
def __init__(self, context, users):
if context.teams_enabled:
self.teams_by_user = {
membership.user.id: membership.team.name
for membership in
CourseTeamMembership.objects.filter(team__course_id=context.course_id, user__in=users)
}
else:
self.teams_by_user = {}
class _EnrollmentBulkContext(object):
def __init__(self, context, users):
CourseEnrollment.bulk_fetch_enrollment_states(users, context.course_id)
self.verified_users = [
verified.user.id for verified in
SoftwareSecurePhotoVerification.verified_query().filter(user__in=users).select_related('user__id')
]
class _CourseGradeBulkContext(object):
def __init__(self, context, users):
self.certs = _CertificateBulkContext(context, users)
self.teams = _TeamBulkContext(context, users)
self.enrollments = _EnrollmentBulkContext(context, users)
bulk_cache_cohorts(context.course_id, users)
BulkRoleCache.prefetch(users)
PersistentCourseGrade.prefetch(context.course_id, users)
BulkCourseTags.prefetch(context.course_id, users)
class CourseGradeReport(object):
"""
Class to encapsulate functionality related to generating Grade Reports.
"""
# Batch size for chunking the list of enrollees in the course.
USER_BATCH_SIZE = 100
@classmethod
def generate(cls, _xmodule_instance_args, _entry_id, course_id, _task_input, action_name):
"""
Public method to generate a grade report.
"""
context = CourseGradeReportContext(_xmodule_instance_args, _entry_id, course_id, _task_input, action_name)
return CourseGradeReport()._generate(context)
with modulestore().bulk_operations(course_id):
context = _CourseGradeReportContext(_xmodule_instance_args, _entry_id, course_id, _task_input, action_name)
return CourseGradeReport()._generate(context)
def _generate(self, context):
"""
@@ -166,6 +222,7 @@ class CourseGradeReport(object):
A generator of batches of (success_rows, error_rows) for this report.
"""
for users in self._batch_users(context):
users = filter(lambda u: u is not None, users)
yield self._rows_for_users(context, users)
def _compile(self, context, batched_rows):
@@ -211,10 +268,11 @@ class CourseGradeReport(object):
"""
Returns a generator of batches of users.
"""
def grouper(iterable, chunk_size=1, fillvalue=None):
def grouper(iterable, chunk_size=self.USER_BATCH_SIZE, fillvalue=None):
args = [iter(iterable)] * chunk_size
return izip_longest(*args, fillvalue=fillvalue)
users = CourseEnrollment.objects.users_enrolled_in(context.course_id)
users = users.select_related('profile__allow_certificate')
return grouper(users)
def _user_grade_results(self, course_grade, context):
@@ -249,7 +307,7 @@ class CourseGradeReport(object):
"""
cohort_group_names = []
if context.cohorts_enabled:
group = get_cohort(user, context.course_id, assign=False)
group = get_cohort(user, context.course_id, assign=False, use_cached=True)
cohort_group_names.append(group.name if group else '')
return cohort_group_names
@@ -264,20 +322,13 @@ class CourseGradeReport(object):
experiment_group_names.append(group.name if group else '')
return experiment_group_names
def _user_team_names(self, user, context):
def _user_team_names(self, user, bulk_teams):
"""
Returns a list of names of teams in which the given user belongs.
"""
team_names = []
if context.teams_enabled:
try:
membership = CourseTeamMembership.objects.get(user=user, team__course_id=context.course_id)
team_names.append(membership.team.name)
except CourseTeamMembership.DoesNotExist:
team_names.append('')
return team_names
return [bulk_teams.teams_by_user.get(user.id, '')]
def _user_verification_mode(self, user, context):
def _user_verification_mode(self, user, context, bulk_enrollments):
"""
Returns a list of enrollment-mode and verification-status for the
given user.
@@ -286,19 +337,21 @@ class CourseGradeReport(object):
verification_status = SoftwareSecurePhotoVerification.verification_status_for_user(
user,
context.course_id,
enrollment_mode
enrollment_mode,
user_is_verified=user.id in bulk_enrollments.verified_users,
)
return [enrollment_mode, verification_status]
def _user_certificate_info(self, user, context, course_grade, whitelisted_user_ids):
def _user_certificate_info(self, user, context, course_grade, bulk_certs):
"""
Returns the course certification information for the given user.
"""
is_whitelisted = user.id in bulk_certs.whitelisted_user_ids
certificate_info = certificate_info_for_user(
user,
context.course_id,
course_grade.letter_grade,
user.id in whitelisted_user_ids
is_whitelisted,
bulk_certs.certificates_by_user.get(user.id),
)
TASK_LOG.info(
u'Student certificate eligibility: %s '
@@ -311,7 +364,7 @@ class CourseGradeReport(object):
course_grade.letter_grade,
context.course.grade_cutoffs,
user.profile.allow_certificate,
user.id in whitelisted_user_ids,
is_whitelisted,
)
return certificate_info
@@ -319,24 +372,30 @@ class CourseGradeReport(object):
"""
Returns a list of rows for the given users for this report.
"""
certificate_whitelist = CertificateWhitelist.objects.filter(course_id=context.course_id, whitelist=True)
whitelisted_user_ids = [entry.user_id for entry in certificate_whitelist]
success_rows, error_rows = [], []
for user, course_grade, error in CourseGradeFactory().iter(users, course_key=context.course_id):
if not course_grade:
# An empty gradeset means we failed to grade a student.
error_rows.append([user.id, user.username, error.message])
else:
success_rows.append(
[user.id, user.email, user.username] +
self._user_grade_results(course_grade, context) +
self._user_cohort_group_names(user, context) +
self._user_experiment_group_names(user, context) +
self._user_team_names(user, context) +
self._user_verification_mode(user, context) +
self._user_certificate_info(user, context, course_grade, whitelisted_user_ids)
)
return success_rows, error_rows
with modulestore().bulk_operations(context.course_id):
bulk_context = _CourseGradeBulkContext(context, users)
success_rows, error_rows = [], []
for user, course_grade, error in CourseGradeFactory().iter(
users,
course=context.course,
collected_block_structure=context.course_structure,
course_key=context.course_id,
):
if not course_grade:
# An empty gradeset means we failed to grade a student.
error_rows.append([user.id, user.username, error.message])
else:
success_rows.append(
[user.id, user.email, user.username] +
self._user_grade_results(course_grade, context) +
self._user_cohort_group_names(user, context) +
self._user_experiment_group_names(user, context) +
self._user_team_names(user, bulk_context.teams) +
self._user_verification_mode(user, context, bulk_context.enrollments) +
self._user_certificate_info(user, context, course_grade, bulk_context.certs)
)
return success_rows, error_rows
class ProblemGradeReport(object):

View File

@@ -34,9 +34,11 @@ from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMe
from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory
from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup, CohortMembership
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory
import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api
from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme
from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent
from request_cache.middleware import RequestCache
from shoppingcart.models import (
Order, PaidCourseRegistration, CourseRegistrationCode, Invoice,
CourseRegistrationCodeInvoiceItem, InvoiceTransaction, Coupon
@@ -44,8 +46,9 @@ from shoppingcart.models import (
from student.models import CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit, ALLOWEDTOENROLL_TO_ENROLLED
from student.tests.factories import CourseEnrollmentFactory, CourseModeFactory, UserFactory
from survey.models import SurveyForm, SurveyAnswer
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
from xmodule.partitions.partitions import Group, UserPartition
from ..models import ReportStore
@@ -321,6 +324,44 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset({'attempted': 1, 'succeeded': 1, 'failed': 0}, result)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 4),
(ModuleStoreEnum.Type.split, 3),
)
@ddt.unpack
def test_query_counts(self, store_type, mongo_count):
with self.store.default_store(store_type):
experiment_group_a = Group(2, u'Expériment Group A')
experiment_group_b = Group(3, u'Expériment Group B')
experiment_partition = UserPartition(
1,
u'Content Expériment Configuration',
u'Group Configuration for Content Expériments',
[experiment_group_a, experiment_group_b],
scheme_id='random'
)
course = CourseFactory.create(
cohort_config={'cohorted': True, 'auto_cohort': True, 'auto_cohort_groups': ['cohort 1', 'cohort 2']},
user_partitions=[experiment_partition],
teams_configuration={
'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}]
},
)
_ = CreditCourseFactory(course_key=course.id)
num_users = 5
for _ in range(num_users):
user = UserFactory.create()
CourseEnrollment.enroll(user, course.id, mode='verified')
SoftwareSecurePhotoVerificationFactory.create(user=user, status='approved')
RequestCache.clear_request_cache()
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
with check_mongo_calls(mongo_count):
with self.assertNumQueries(41):
CourseGradeReport.generate(None, None, course.id, None, 'graded')
class TestTeamGradeReport(InstructorGradeReportTestCase):
""" Test that teams appear correctly in the grade report when it is enabled for the course. """
@@ -1783,7 +1824,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
'failed': 3,
'skipped': 2
}
with self.assertNumQueries(186):
with self.assertNumQueries(171):
self.assertCertificatesGenerated(task_input, expected_results)
expected_results = {

View File

@@ -210,12 +210,19 @@ class PhotoVerification(StatusModel):
This will check for the user's *initial* verification.
"""
return cls.verified_query(earliest_allowed_date).filter(user=user).exists()
@classmethod
def verified_query(cls, earliest_allowed_date=None):
"""
Return a query set for all records with 'approved' state
that are still valid according to the earliest_allowed_date
value or policy settings.
"""
return cls.objects.filter(
user=user,
status="approved",
created_at__gte=(earliest_allowed_date
or cls._earliest_allowed_date())
).exists()
created_at__gte=(earliest_allowed_date or cls._earliest_allowed_date()),
)
@classmethod
def verification_valid_or_pending(cls, user, earliest_allowed_date=None, queryset=None):
@@ -951,14 +958,15 @@ class SoftwareSecurePhotoVerification(PhotoVerification):
return response
@classmethod
def verification_status_for_user(cls, user, course_id, user_enrollment_mode):
def verification_status_for_user(cls, user, course_id, user_enrollment_mode, user_is_verified=None):
"""
Returns the verification status for use in grade report.
"""
if user_enrollment_mode not in CourseMode.VERIFIED_MODES:
return 'N/A'
user_is_verified = cls.user_is_verified(user)
if user_is_verified is None:
user_is_verified = cls.user_is_verified(user)
if not user_is_verified:
return 'Not ID Verified'