Skip to content

E
edx-platform
Commit 06ddb2f3 by Will Daly
Options

Credit requirement optimizations 

* Skip adding the credit req tasks to the queue for non-credit courses.
* Load only ICRV XBlocks instead of traversing the course tree.
parent 0bb519c2
Showing with 82 additions and 58 deletions
openedx/core/djangoapps/credit/signals.py
......@@ -2,6 +2,8 @@
This file contains receivers of course publication signals.
"""
import logging
from django.dispatch import receiver
from django.utils import timezone
from opaque_keys.edx.keys import CourseKey
......@@ -10,6 +12,9 @@ from xmodule.modulestore.django import SignalHandler
from openedx.core.djangoapps.signals.signals import GRADES_UPDATED
log = logging.getLogger(__name__)
@receiver(SignalHandler.course_published)
def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument
"""Receive 'course_published' signal and kick off a celery task to update
......@@ -18,9 +23,11 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=
# Import here, because signal is registered at startup, but items in tasks
# are not yet able to be loaded
from .tasks import update_credit_course_requirements
from openedx.core.djangoapps.credit import api, tasks
update_credit_course_requirements.delay(unicode(course_key))
if api.is_credit_course(course_key):
tasks.update_credit_course_requirements.delay(unicode(course_key))
log.info(u'Added task to update credit requirements for course "%s" to the task queue', course_key)
@receiver(GRADES_UPDATED)
......
openedx/core/djangoapps/credit/tasks.py
......@@ -18,10 +18,20 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
LOGGER = get_task_logger(__name__)
# XBlocks that can be added as credit requirements
CREDIT_REQUIREMENT_XBLOCKS = [
{
"category": "edx-reverification-block",
"requirement-namespace": "reverification"
}
]
# pylint: disable=not-callable
@task(default_retry_delay=settings.CREDIT_TASK_DEFAULT_RETRY_DELAY, max_retries=settings.CREDIT_TASK_MAX_RETRIES)
def update_credit_course_requirements(course_id): # pylint: disable=invalid-name
"""Updates course requirements table for a course.
"""
Updates course requirements table for a course.
Args:
course_id(str): A string representation of course identifier
......@@ -34,8 +44,7 @@ def update_credit_course_requirements(course_id): # pylint: disable=invalid-na
course_key = CourseKey.from_string(course_id)
is_credit_course = CreditCourse.is_credit_course(course_key)
if is_credit_course:
course = modulestore().get_course(course_key)
requirements = _get_course_credit_requirements(course)
requirements = _get_course_credit_requirements(course_key)
set_credit_requirements(course_key, requirements)
except (InvalidKeyError, ItemNotFoundError, InvalidCreditRequirements) as exc:
LOGGER.error('Error on adding the requirements for course %s - %s', course_id, unicode(exc))
......@@ -44,41 +53,40 @@ def update_credit_course_requirements(course_id): # pylint: disable=invalid-na
LOGGER.info('Requirements added for course %s', course_id)
def _get_course_credit_requirements(course):
"""Returns the list of credit requirements for the given course.
def _get_course_credit_requirements(course_key):
"""
Returns the list of credit requirements for the given course.
It returns the minimum_grade_credit and also the ICRV checkpoints
if any were added in the course
Args:
course(Course): The course object
course_key (CourseKey): Identifier for the course.
Returns:
List of minimum_grade_credit and ICRV requirements
List of credit requirements (dictionaries)
"""
credit_xblock_requirements = _get_credit_course_requirement_xblocks(course)
min_grade_requirement = _get_min_grade_requirement(course)
credit_xblock_requirements = _get_credit_course_requirement_xblocks(course_key)
min_grade_requirement = _get_min_grade_requirement(course_key)
credit_requirements = min_grade_requirement + credit_xblock_requirements
return credit_requirements
def _get_min_grade_requirement(course):
"""Get list of 'minimum_grade_credit' requirement for the given course.
def _get_min_grade_requirement(course_key):
"""
Get list of 'minimum_grade_credit' requirement for the given course.
Args:
course(Course): The course object
Raises:
AttributeError if the course has not 'minimum_grade_credit' attribute
course_key (CourseKey): Identifier for the course.
Returns:
The list of minimum_grade_credit requirements
"""
requirement = []
course = modulestore().get_course(course_key, depth=0)
try:
requirement = [
return [
{
"namespace": "grade",
"name": "grade",
......@@ -90,41 +98,46 @@ def _get_min_grade_requirement(course):
]
except AttributeError:
LOGGER.error("The course %s does not has minimum_grade_credit attribute", unicode(course.id))
return requirement
else:
return []
def _get_credit_course_requirement_xblocks(course): # pylint: disable=invalid-name
def _get_credit_course_requirement_xblocks(course_key): # pylint: disable=invalid-name
"""Generate a course structure dictionary for the specified course.
Args:
course(Course): The course object
course_key (CourseKey): Identifier for the course.
Returns:
The list of credit requirements xblocks dicts
"""
blocks_stack = [course]
requirements_blocks = []
while blocks_stack:
curr_block = blocks_stack.pop()
children = curr_block.get_children() if curr_block.has_children else []
if _is_credit_requirement(curr_block):
block = {
"namespace": curr_block.get_credit_requirement_namespace(),
"name": curr_block.get_credit_requirement_name(),
"display_name": curr_block.get_credit_requirement_display_name(),
"criteria": "",
requirements = []
# Retrieve all XBlocks from the course that we know to be credit requirements.
# For performance reasons, we look these up by their "category" to avoid
# loading and searching the entire course tree.
for desc in CREDIT_REQUIREMENT_XBLOCKS:
requirements.extend([
{
"namespace": desc["requirement-namespace"],
"name": block.get_credit_requirement_name(),
"display_name": block.get_credit_requirement_display_name(),
"criteria": {},
}
requirements_blocks.append(block)
for block in modulestore().get_items(
course_key,
qualifiers={"category": desc["category"]}
)
if _is_credit_requirement(block)
])
# Add the children of current block to the stack so that we can
# traverse them as well.
blocks_stack.extend(children)
return requirements_blocks
return requirements
def _is_credit_requirement(xblock):
"""Check if the given XBlock is a credit requirement.
"""
Check if the given XBlock is a credit requirement.
Args:
xblock(XBlock): The given XBlock object
......@@ -133,21 +146,18 @@ def _is_credit_requirement(xblock):
True if XBlock is a credit requirement else False
"""
is_credit_requirement = False
if callable(getattr(xblock, "is_course_credit_requirement", None)):
is_credit_requirement = xblock.is_course_credit_requirement()
if is_credit_requirement:
if not callable(getattr(xblock, "get_credit_requirement_namespace", None)):
is_credit_requirement = False
LOGGER.error(
"XBlock %s is marked as a credit requirement but does not "
"implement get_credit_requirement_namespace()", unicode(xblock)
)
if not callable(getattr(xblock, "get_credit_requirement_name", None)):
is_credit_requirement = False
LOGGER.error(
"XBlock %s is marked as a credit requirement but does not "
"implement get_credit_requirement_name()", unicode(xblock)
)
return is_credit_requirement
if not callable(getattr(xblock, "get_credit_requirement_namespace", None)):
LOGGER.error(
"XBlock %s is marked as a credit requirement but does not "
"implement get_credit_requirement_namespace()", unicode(xblock)
)
return False
if not callable(getattr(xblock, "get_credit_requirement_name", None)):
LOGGER.error(
"XBlock %s is marked as a credit requirement but does not "
"implement get_credit_requirement_name()", unicode(xblock)
)
return False
return True
openedx/core/djangoapps/credit/tests/test_tasks.py
......@@ -11,7 +11,7 @@ from openedx.core.djangoapps.credit.models import CreditCourse
from openedx.core.djangoapps.credit.signals import listen_for_course_publish
from xmodule.modulestore.django import SignalHandler
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
class TestTaskExecution(ModuleStoreTestCase):
......@@ -85,6 +85,13 @@ class TestTaskExecution(ModuleStoreTestCase):
requirements = get_credit_requirements(self.course.id)
self.assertEqual(len(requirements), 2)
def test_query_counts(self):
self.add_credit_course(self.course.id)
self.add_icrv_xblock()
with check_mongo_calls(3):
listen_for_course_publish(self, self.course.id)
@mock.patch(
'openedx.core.djangoapps.credit.tasks.set_credit_requirements',
mock.Mock(
......
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