From 952143b7767c20aa64c527f238a5d664f3d06462 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 1 Mar 2017 21:12:49 -0500 Subject: [PATCH] Block Structure Storage management command TNL-6524 --- .../commands/generate_course_blocks.py | 72 +++++++++++++------ .../tests/test_generate_course_blocks.py | 29 +++++++- .../content/block_structure/tests/helpers.py | 13 ++++ 3 files changed, 88 insertions(+), 26 deletions(-) diff --git a/lms/djangoapps/course_blocks/management/commands/generate_course_blocks.py b/lms/djangoapps/course_blocks/management/commands/generate_course_blocks.py index 6dd9c895d7..4f0d056ed7 100644 --- a/lms/djangoapps/course_blocks/management/commands/generate_course_blocks.py +++ b/lms/djangoapps/course_blocks/management/commands/generate_course_blocks.py @@ -7,6 +7,7 @@ from django.core.management.base import BaseCommand from xmodule.modulestore.django import modulestore import openedx.core.djangoapps.content.block_structure.api as api +from openedx.core.djangoapps.content.block_structure.config import _bs_waffle_switch_name, STORAGE_BACKING_FOR_CACHE import openedx.core.djangoapps.content.block_structure.tasks as tasks import openedx.core.djangoapps.content.block_structure.store as store from openedx.core.lib.command_utils import ( @@ -14,6 +15,8 @@ from openedx.core.lib.command_utils import ( validate_dependent_option, parse_course_keys, ) +from request_cache.middleware import RequestCache, func_call_cache_key +from openedx.core.djangolib.waffle_utils import is_switch_enabled log = logging.getLogger(__name__) @@ -25,8 +28,8 @@ class Command(BaseCommand): $ ./manage.py lms generate_course_blocks --all --settings=devstack $ ./manage.py lms generate_course_blocks 'edX/DemoX/Demo_Course' --settings=devstack """ - args = '' - help = 'Generates and stores course blocks for one or more courses.' + args = u'' + help = u'Generates and stores course blocks for one or more courses.' def add_arguments(self, parser): """ @@ -36,43 +39,49 @@ class Command(BaseCommand): '--courses', dest='courses', nargs='+', - help='Generate course blocks for the list of courses provided.', + help=u'Generate course blocks for the list of courses provided.', ) parser.add_argument( '--all_courses', - help='Generate course blocks for all courses, given the requested start and end indices.', + help=u'Generate course blocks for all courses, given the requested start and end indices.', action='store_true', default=False, ) parser.add_argument( '--enqueue_task', - help='Enqueue the tasks for asynchronous computation.', + help=u'Enqueue the tasks for asynchronous computation.', action='store_true', default=False, ) parser.add_argument( '--routing_key', dest='routing_key', - help='Routing key to use for asynchronous computation.', + help=u'Routing key to use for asynchronous computation.', ) parser.add_argument( '--force_update', - help='Force update of the course blocks for the requested courses.', + help=u'Force update of the course blocks for the requested courses.', action='store_true', default=False, ) parser.add_argument( '--start_index', - help='Starting index of course list.', + help=u'Starting index of course list.', default=0, type=int, ) parser.add_argument( '--end_index', - help='Ending index of course list.', + help=u'Ending index of course list.', default=0, type=int, ) + parser.add_argument( + '--with_storage', + help=u'Store the course blocks in Storage, overriding value of the storage_backing_for_cache waffle switch', + action='store_true', + default=False, + ) def handle(self, *args, **options): @@ -91,9 +100,9 @@ class Command(BaseCommand): self._set_log_levels(options) - log.warning('STARTED generating Course Blocks for %d courses.', len(course_keys)) + log.critical(u'STARTED generating Course Blocks for %d courses.', len(course_keys)) self._generate_course_blocks(options, course_keys) - log.warning('FINISHED generating Course Blocks for %d courses.', len(course_keys)) + log.critical(u'FINISHED generating Course Blocks for %d courses.', len(course_keys)) def _set_log_levels(self, options): """ @@ -119,23 +128,40 @@ class Command(BaseCommand): """ Generates course blocks for the given course_keys per the given options. """ + if options.get('with_storage'): + self._enable_storage() for course_key in course_keys: try: - log.info('STARTED generating Course Blocks for course: %s.', course_key) - - if options.get('enqueue_task'): - action = tasks.update_course_in_cache if options.get('force_update') else tasks.get_course_in_cache - task_options = {'routing_key': options['routing_key']} if options.get('routing_key') else {} - action.apply_async([unicode(course_key)], **task_options) - else: - action = api.update_course_in_cache if options.get('force_update') else api.get_course_in_cache - action(course_key) - - log.info('FINISHED generating Course Blocks for course: %s.', course_key) + log.info(u'STARTED generating Course Blocks for course: %s.', course_key) + self._generate_for_course(options, course_key) + log.info(u'FINISHED generating Course Blocks for course: %s.', course_key) except Exception as ex: # pylint: disable=broad-except log.exception( - 'An error occurred while generating course blocks for %s: %s', + u'An error occurred while generating course blocks for %s: %s', unicode(course_key), ex.message, ) + + def _generate_for_course(self, options, course_key): + """ + Generates course blocks for the given course_key per the given options. + """ + if options.get('enqueue_task'): + action = tasks.update_course_in_cache if options.get('force_update') else tasks.get_course_in_cache + task_options = {'routing_key': options['routing_key']} if options.get('routing_key') else {} + action.apply_async([unicode(course_key)], **task_options) + else: + action = api.update_course_in_cache if options.get('force_update') else api.get_course_in_cache + action(course_key) + + def _enable_storage(self): + """ + Enables storage backing by setting the waffle's cached value to True. + """ + cache_key = func_call_cache_key( + is_switch_enabled.request_cached_contained_func, + _bs_waffle_switch_name(STORAGE_BACKING_FOR_CACHE), + ) + RequestCache.get_request_cache().data[cache_key] = True + log.warning(u'STORAGE_BACKING_FOR_CACHE is enabled.') diff --git a/lms/djangoapps/course_blocks/management/commands/tests/test_generate_course_blocks.py b/lms/djangoapps/course_blocks/management/commands/tests/test_generate_course_blocks.py index 25b49da7c7..b2c873ab82 100644 --- a/lms/djangoapps/course_blocks/management/commands/tests/test_generate_course_blocks.py +++ b/lms/djangoapps/course_blocks/management/commands/tests/test_generate_course_blocks.py @@ -6,11 +6,13 @@ from django.core.management.base import CommandError import itertools from mock import patch -from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.factories import CourseFactory from .. import generate_course_blocks -from openedx.core.djangoapps.content.block_structure.tests.helpers import is_course_in_block_structure_cache +from openedx.core.djangoapps.content.block_structure.tests.helpers import ( + is_course_in_block_structure_cache, + is_course_in_block_structure_storage, +) @ddt.ddt @@ -43,6 +45,20 @@ class TestGenerateCourseBlocks(ModuleStoreTestCase): for course_key in course_keys: self.assertTrue(is_course_in_block_structure_cache(course_key, self.store)) + def _assert_courses_not_in_block_storage(self, *course_keys): + """ + Assert courses don't exist in course block storage. + """ + for course_key in course_keys: + self.assertFalse(is_course_in_block_structure_storage(course_key, self.store)) + + def _assert_courses_in_block_storage(self, *course_keys): + """ + Assert courses exist in course block storage. + """ + for course_key in course_keys: + self.assertTrue(is_course_in_block_structure_storage(course_key, self.store)) + def _assert_message_presence_in_logs(self, message, mock_log, expected_presence=True): """ Asserts that the logger was called with the given message. @@ -69,6 +85,13 @@ class TestGenerateCourseBlocks(ModuleStoreTestCase): self.command.handle(courses=[unicode(self.course_keys[0])]) self._assert_courses_in_block_cache(self.course_keys[0]) self._assert_courses_not_in_block_cache(*self.course_keys[1:]) + self._assert_courses_not_in_block_storage(*self.course_keys) + + def test_with_storage(self): + self.command.handle(with_storage=True, courses=[unicode(self.course_keys[0])]) + self._assert_courses_in_block_cache(self.course_keys[0]) + self._assert_courses_in_block_storage(self.course_keys[0]) + self._assert_courses_not_in_block_storage(*self.course_keys[1:]) @ddt.data( *itertools.product( diff --git a/openedx/core/djangoapps/content/block_structure/tests/helpers.py b/openedx/core/djangoapps/content/block_structure/tests/helpers.py index 07c3dba4bd..6ab1ab7e3a 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/helpers.py +++ b/openedx/core/djangoapps/content/block_structure/tests/helpers.py @@ -13,6 +13,7 @@ from ..api import get_cache from ..block_structure import BlockStructureBlockData from ..config import _bs_waffle_switch_name from ..exceptions import BlockStructureNotFound +from ..models import BlockStructureModel from ..store import BlockStructureStore from ..transformer import BlockStructureTransformer, FilteringTransformerMixin from ..transformer_registry import TransformerRegistry @@ -30,6 +31,18 @@ def is_course_in_block_structure_cache(course_key, store): return False +def is_course_in_block_structure_storage(course_key, store): + """ + Returns whether the given course is in Block Structure storage. + """ + course_usage_key = store.make_course_usage_key(course_key) + try: + BlockStructureModel.get(course_usage_key) + return True + except BlockStructureNotFound: + return False + + class override_config_setting(override_switch): # pylint:disable=invalid-name """ Subclasses override_switch to use the block structure