Skip to content

E
edx-platform
Commit a1778a17 by David Ormsbee
Options

Revert "more granular transactions in grading [LMS-1480]" 

This reverts commit 19dd49f3.
parent 371f4c9d
Showing with 73 additions and 121 deletions
lms/djangoapps/courseware/grades.py
......@@ -4,11 +4,9 @@ from __future__ import division
import random
import logging
from contextlib import contextmanager
from collections import defaultdict
from django.conf import settings
from django.contrib.auth.models import User
from django.db import transaction
from courseware.model_data import FieldDataCache, DjangoKeyValueStore
from xblock.fields import Scope
......@@ -123,20 +121,8 @@ def answer_distributions(request, course):
return counts
@transaction.commit_manually
def grade(student, request, course, keep_raw_scores=False):
def grade(student, request, course, field_data_cache=None, keep_raw_scores=False):
"""
Wraps "_grade" with the manual_transaction context manager just in case
there are unanticipated errors.
"""
with manual_transaction():
return _grade(student, request, course, keep_raw_scores)
def _grade(student, request, course, keep_raw_scores):
"""
Unwrapped version of "grade"
This grades a student as quickly as possible. It returns the
output from the course grader, augmented with the final letter
grade. The keys in the output are:
......@@ -149,14 +135,17 @@ def _grade(student, request, course, keep_raw_scores):
up the grade. (For display)
- grade_breakdown : A breakdown of the major components that
make up the final grade. (For display)
- keep_raw_scores : if True, then value for key 'raw_scores' contains scores
for every graded module
- keep_raw_scores : if True, then value for key 'raw_scores' contains scores for every graded module
More information on the format is in the docstring for CourseGrader.
"""
grading_context = course.grading_context
raw_scores = []
if field_data_cache is None:
field_data_cache = FieldDataCache(grading_context['all_descriptors'], course.id, student)
totaled_scores = {}
# This next complicated loop is just to collect the totaled_scores, which is
# passed to the grader
......@@ -166,22 +155,26 @@ def _grade(student, request, course, keep_raw_scores):
section_descriptor = section['section_descriptor']
section_name = section_descriptor.display_name_with_default
should_grade_section = False
# If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0%
for moduledescriptor in section['xmoduledescriptors']:
# some problems have state that is updated independently of interaction
# with the LMS, so they need to always be scored. (E.g. foldit.)
should_grade_section = any(
descriptor.always_recalculate_grades for descriptor in section['xmoduledescriptors']
)
if moduledescriptor.always_recalculate_grades:
should_grade_section = True
break
# If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0%
if not should_grade_section:
with manual_transaction():
should_grade_section = StudentModule.objects.filter(
student=student,
module_type='problem',
module_state_key__in=[
descriptor.location for descriptor in section['xmoduledescriptors']
]
).exists()
# Create a fake key to pull out a StudentModule object from the FieldDataCache
key = DjangoKeyValueStore.Key(
Scope.user_state,
student.id,
moduledescriptor.location,
None
)
if field_data_cache.find(key):
should_grade_section = True
break
if should_grade_section:
scores = []
......@@ -190,13 +183,11 @@ def _grade(student, request, course, keep_raw_scores):
'''creates an XModule instance given a descriptor'''
# TODO: We need the request to pass into here. If we could forego that, our arguments
# would be simpler
with manual_transaction():
field_data_cache = FieldDataCache([descriptor], course.id, student)
return get_module_for_descriptor(student, request, descriptor, field_data_cache, course.id)
for module_descriptor in yield_dynamic_descriptor_descendents(section_descriptor, create_module):
(correct, total) = get_score(course.id, student, module_descriptor, create_module)
(correct, total) = get_score(course.id, student, module_descriptor, create_module, field_data_cache)
if correct is None and total is None:
continue
......@@ -265,23 +256,11 @@ def grade_for_percentage(grade_cutoffs, percentage):
return letter_grade
@transaction.commit_manually
def progress_summary(student, request, course):
"""
Wraps "_progress_summary" with the manual_transaction context manager just
in case there are unanticipated errors.
"""
with manual_transaction():
return _progress_summary(student, request, course)
# TODO: This method is not very good. It was written in the old course style and
# then converted over and performance is not good. Once the progress page is redesigned
# to not have the progress summary this method should be deleted (so it won't be copied).
def _progress_summary(student, request, course):
def progress_summary(student, request, course, field_data_cache):
"""
Unwrapped version of "progress_summary".
This pulls a summary of all problems in the course.
Returns
......@@ -294,17 +273,16 @@ def _progress_summary(student, request, course):
Arguments:
student: A User object for the student to grade
course: A Descriptor containing the course to grade
field_data_cache: A FieldDataCache initialized with all
instance_modules for the student
If the student does not have access to load the course module, this function
will return None.
"""
with manual_transaction():
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
course.id, student, course, depth=None
)
# TODO: We need the request to pass into here. If we could
# forego that, our arguments would be simpler
# TODO: We need the request to pass into here. If we could forego that, our arguments
# would be simpler
course_module = get_module_for_descriptor(student, request, course, field_data_cache, course.id)
if not course_module:
# This student must not have access to the course.
......@@ -318,13 +296,12 @@ def _progress_summary(student, request, course):
continue
sections = []
for section_module in chapter_module.get_display_items():
# Skip if the section is hidden
with manual_transaction():
if section_module.hide_from_toc:
continue
# Same for sections
graded = section_module.graded
scores = []
......@@ -333,7 +310,7 @@ def _progress_summary(student, request, course):
for module_descriptor in yield_dynamic_descriptor_descendents(section_module, module_creator):
course_id = course.id
(correct, total) = get_score(course_id, student, module_descriptor, module_creator)
(correct, total) = get_score(course_id, student, module_descriptor, module_creator, field_data_cache)
if correct is None and total is None:
continue
......@@ -354,16 +331,15 @@ def _progress_summary(student, request, course):
'graded': graded,
})
chapters.append({
'course': course.display_name_with_default,
chapters.append({'course': course.display_name_with_default,
'display_name': chapter_module.display_name_with_default,
'url_name': chapter_module.url_name,
'sections': sections
})
'sections': sections})
return chapters
def get_score(course_id, user, problem_descriptor, module_creator):
def get_score(course_id, user, problem_descriptor, module_creator, field_data_cache):
"""
Return the score for a user on a problem, as a tuple (correct, total).
e.g. (5,7) if you got 5 out of 7 points.
......@@ -396,15 +372,15 @@ def get_score(course_id, user, problem_descriptor, module_creator):
# These are not problems, and do not have a score
return (None, None)
try:
student_module = StudentModule.objects.get(
student=user,
course_id=course_id,
module_type='problem',
module_state_key=problem_descriptor.location
# Create a fake KeyValueStore key to pull out the StudentModule
key = DjangoKeyValueStore.Key(
Scope.user_state,
user.id,
problem_descriptor.location,
None
)
except StudentModule.DoesNotExist:
student_module = None
student_module = field_data_cache.find(key)
if student_module is not None and student_module.max_grade is not None:
correct = student_module.grade if student_module.grade is not None else 0
......@@ -435,16 +411,3 @@ def get_score(course_id, user, problem_descriptor, module_creator):
total = weight
return (correct, total)
@contextmanager
def manual_transaction():
"""A context manager for managing manual transactions"""
try:
yield
except Exception:
transaction.rollback()
log.exception('Due to an error, this transaction has been rolled back')
raise
else:
transaction.commit()
lms/djangoapps/courseware/tests/test_submitting_problems.py
......@@ -236,11 +236,14 @@ class TestCourseGrader(TestSubmittingProblems):
make up the final grade. (For display)
"""
fake_request = self.factory.get(
reverse('progress', kwargs={'course_id': self.course.id})
)
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
self.course.id, self.student_user, self.course)
fake_request = self.factory.get(reverse('progress',
kwargs={'course_id': self.course.id}))
return grades.grade(self.student_user, fake_request, self.course)
return grades.grade(self.student_user, fake_request,
self.course, field_data_cache)
def get_progress_summary(self):
"""
......@@ -254,13 +257,16 @@ class TestCourseGrader(TestSubmittingProblems):
etc.
"""
fake_request = self.factory.get(
reverse('progress', kwargs={'course_id': self.course.id})
)
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
self.course.id, self.student_user, self.course)
progress_summary = grades.progress_summary(
self.student_user, fake_request, self.course
)
fake_request = self.factory.get(reverse('progress',
kwargs={'course_id': self.course.id}))
progress_summary = grades.progress_summary(self.student_user,
fake_request,
self.course,
field_data_cache)
return progress_summary
def check_grade_percent(self, percent):
......
lms/djangoapps/courseware/views.py
......@@ -14,7 +14,6 @@ from django.shortcuts import redirect
from mitxmako.shortcuts import render_to_response, render_to_string
from django_future.csrf import ensure_csrf_cookie
from django.views.decorators.cache import cache_control
from django.db import transaction
from markupsafe import escape
from courseware import grades
......@@ -644,9 +643,8 @@ def mktg_course_about(request, course_id):
except (ValueError, Http404) as e:
# if a course does not exist yet, display a coming
# soon button
return render_to_response(
'courseware/mktg_coming_soon.html', {'course_id': course_id}
)
return render_to_response('courseware/mktg_coming_soon.html',
{'course_id': course_id})
registered = registered_for_course(course, request.user)
......@@ -661,8 +659,7 @@ def mktg_course_about(request, course_id):
settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'))
course_modes = CourseMode.modes_for_course(course.id)
return render_to_response(
'courseware/mktg_course_about.html',
return render_to_response('courseware/mktg_course_about.html',
{
'course': course,
'registered': registered,
......@@ -670,27 +667,13 @@ def mktg_course_about(request, course_id):
'course_target': course_target,
'show_courseware_link': show_courseware_link,
'course_modes': course_modes,
}
)
})
@login_required
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@transaction.commit_manually
def progress(request, course_id, student_id=None):
"""
Wraps "_progress" with the manual_transaction context manager just in case
there are unanticipated errors.
"""
with grades.manual_transaction():
return _progress(request, course_id, student_id)
def _progress(request, course_id, student_id):
"""
Unwrapped version of "progress".
User progress. We show the grade bar and every problem score.
""" User progress. We show the grade bar and every problem score.
Course staff are allowed to see the progress of students in their class.
"""
......@@ -713,26 +696,26 @@ def _progress(request, course_id, student_id):
# additional DB lookup (this kills the Progress page in particular).
student = User.objects.prefetch_related("groups").get(id=student.id)
courseware_summary = grades.progress_summary(student, request, course)
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
course_id, student, course, depth=None)
grade_summary = grades.grade(student, request, course)
courseware_summary = grades.progress_summary(student, request, course,
field_data_cache)
grade_summary = grades.grade(student, request, course, field_data_cache)
if courseware_summary is None:
#This means the student didn't have access to the course (which the instructor requested)
raise Http404
context = {
'course': course,
context = {'course': course,
'courseware_summary': courseware_summary,
'grade_summary': grade_summary,
'staff_access': staff_access,
'student': student,
}
context.update()
with grades.manual_transaction():
response = render_to_response('courseware/progress.html', context)
return response
return render_to_response('courseware/progress.html', context)
@login_required
......
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