Merge pull request #9478 from edx/mjevtic/SOL-1105
(SOL-1154) Enabling search in devstack
This commit is contained in:
@@ -0,0 +1,115 @@
|
||||
""" Management command to update courses' search index """
|
||||
import logging
|
||||
from django.core.management import BaseCommand, CommandError
|
||||
from optparse import make_option
|
||||
from textwrap import dedent
|
||||
|
||||
from contentstore.courseware_index import CoursewareSearchIndexer
|
||||
from search.search_engine_base import SearchEngine
|
||||
from elasticsearch import exceptions
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.locator import CourseLocator
|
||||
|
||||
from .prompt import query_yes_no
|
||||
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
"""
|
||||
Command to re-index courses
|
||||
|
||||
Examples:
|
||||
|
||||
./manage.py reindex_course <course_id_1> <course_id_2> - reindexes courses with keys course_id_1 and course_id_2
|
||||
./manage.py reindex_course --all - reindexes all available courses
|
||||
./manage.py reindex_course --setup - reindexes all courses for devstack setup
|
||||
"""
|
||||
help = dedent(__doc__)
|
||||
|
||||
can_import_settings = True
|
||||
|
||||
args = "<course_id course_id ...>"
|
||||
|
||||
all_option = make_option('--all',
|
||||
action='store_true',
|
||||
dest='all',
|
||||
default=False,
|
||||
help='Reindex all courses')
|
||||
|
||||
setup_option = make_option('--setup',
|
||||
action='store_true',
|
||||
dest='setup',
|
||||
default=False,
|
||||
help='Reindex all courses on developers stack setup')
|
||||
|
||||
option_list = BaseCommand.option_list + (all_option, setup_option)
|
||||
|
||||
CONFIRMATION_PROMPT = u"Re-indexing all courses might be a time consuming operation. Do you want to continue?"
|
||||
|
||||
def _parse_course_key(self, raw_value):
|
||||
""" Parses course key from string """
|
||||
try:
|
||||
result = CourseKey.from_string(raw_value)
|
||||
except InvalidKeyError:
|
||||
raise CommandError("Invalid course_key: '%s'." % raw_value)
|
||||
|
||||
if not isinstance(result, CourseLocator):
|
||||
raise CommandError(u"Argument {0} is not a course key".format(raw_value))
|
||||
|
||||
return result
|
||||
|
||||
def handle(self, *args, **options):
|
||||
"""
|
||||
By convention set by Django developers, this method actually executes command's actions.
|
||||
So, there could be no better docstring than emphasize this once again.
|
||||
"""
|
||||
all_option = options.get('all', False)
|
||||
setup_option = options.get('setup', False)
|
||||
index_all_courses_option = all_option or setup_option
|
||||
|
||||
if len(args) == 0 and not index_all_courses_option:
|
||||
raise CommandError(u"reindex_course requires one or more arguments: <course_id>")
|
||||
|
||||
store = modulestore()
|
||||
|
||||
if index_all_courses_option:
|
||||
index_name = CoursewareSearchIndexer.INDEX_NAME
|
||||
doc_type = CoursewareSearchIndexer.DOCUMENT_TYPE
|
||||
if setup_option:
|
||||
try:
|
||||
# try getting the ElasticSearch engine
|
||||
searcher = SearchEngine.get_search_engine(index_name)
|
||||
except exceptions.ElasticsearchException as exc:
|
||||
logging.exception('Search Engine error - %s', unicode(exc))
|
||||
return
|
||||
|
||||
index_exists = searcher._es.indices.exists(index=index_name) # pylint: disable=protected-access
|
||||
doc_type_exists = searcher._es.indices.exists_type( # pylint: disable=protected-access
|
||||
index=index_name,
|
||||
doc_type=doc_type
|
||||
)
|
||||
|
||||
index_mapping = searcher._es.indices.get_mapping( # pylint: disable=protected-access
|
||||
index=index_name,
|
||||
doc_type=doc_type
|
||||
) if index_exists and doc_type_exists else {}
|
||||
|
||||
if index_exists and index_mapping:
|
||||
return
|
||||
|
||||
# if reindexing is done during devstack setup step, don't prompt the user
|
||||
if setup_option or query_yes_no(self.CONFIRMATION_PROMPT, default="no"):
|
||||
# in case of --setup or --all, get the list of course keys from all courses
|
||||
# that are stored in the modulestore
|
||||
course_keys = [course.id for course in modulestore().get_courses()]
|
||||
else:
|
||||
return
|
||||
else:
|
||||
# in case course keys are provided as arguments
|
||||
course_keys = map(self._parse_course_key, args)
|
||||
|
||||
for course_key in course_keys:
|
||||
CoursewareSearchIndexer.do_course_reindex(store, course_key)
|
||||
@@ -0,0 +1,131 @@
|
||||
""" Tests for course reindex command """
|
||||
import ddt
|
||||
from django.core.management import call_command, CommandError
|
||||
import mock
|
||||
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from common.test.utils import nostderr
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory
|
||||
|
||||
from contentstore.management.commands.reindex_course import Command as ReindexCommand
|
||||
from contentstore.courseware_index import SearchIndexingError
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class TestReindexCourse(ModuleStoreTestCase):
|
||||
""" Tests for course reindex command """
|
||||
def setUp(self):
|
||||
""" Setup method - create courses """
|
||||
super(TestReindexCourse, self).setUp()
|
||||
self.store = modulestore()
|
||||
self.first_lib = LibraryFactory.create(
|
||||
org="test", library="lib1", display_name="run1", default_store=ModuleStoreEnum.Type.split
|
||||
)
|
||||
self.second_lib = LibraryFactory.create(
|
||||
org="test", library="lib2", display_name="run2", default_store=ModuleStoreEnum.Type.split
|
||||
)
|
||||
|
||||
self.first_course = CourseFactory.create(
|
||||
org="test", course="course1", display_name="run1"
|
||||
)
|
||||
self.second_course = CourseFactory.create(
|
||||
org="test", course="course2", display_name="run1"
|
||||
)
|
||||
|
||||
REINDEX_PATH_LOCATION = 'contentstore.management.commands.reindex_course.CoursewareSearchIndexer.do_course_reindex'
|
||||
MODULESTORE_PATCH_LOCATION = 'contentstore.management.commands.reindex_course.modulestore'
|
||||
YESNO_PATCH_LOCATION = 'contentstore.management.commands.reindex_course.query_yes_no'
|
||||
|
||||
def _get_lib_key(self, library):
|
||||
""" Get's library key as it is passed to indexer """
|
||||
return library.location.library_key
|
||||
|
||||
def _build_calls(self, *courses):
|
||||
""" Builds a list of mock.call instances representing calls to reindexing method """
|
||||
return [mock.call(self.store, course.id) for course in courses]
|
||||
|
||||
def test_given_no_arguments_raises_command_error(self):
|
||||
""" Test that raises CommandError for incorrect arguments """
|
||||
with self.assertRaises(SystemExit), nostderr():
|
||||
with self.assertRaisesRegexp(CommandError, ".* requires one or more arguments .*"):
|
||||
call_command('reindex_course')
|
||||
|
||||
@ddt.data('qwerty', 'invalid_key', 'xblock-v1:qwe+rty')
|
||||
def test_given_invalid_course_key_raises_not_found(self, invalid_key):
|
||||
""" Test that raises InvalidKeyError for invalid keys """
|
||||
errstring = "Invalid course_key: '%s'." % invalid_key
|
||||
with self.assertRaises(SystemExit) as ex:
|
||||
with self.assertRaisesRegexp(CommandError, errstring):
|
||||
call_command('reindex_course', invalid_key)
|
||||
self.assertEqual(ex.exception.code, 1)
|
||||
|
||||
def test_given_library_key_raises_command_error(self):
|
||||
""" Test that raises CommandError if library key is passed """
|
||||
with self.assertRaises(SystemExit), nostderr():
|
||||
with self.assertRaisesRegexp(SearchIndexingError, ".* is not a course key"):
|
||||
call_command('reindex_course', unicode(self._get_lib_key(self.first_lib)))
|
||||
|
||||
with self.assertRaises(SystemExit), nostderr():
|
||||
with self.assertRaisesRegexp(SearchIndexingError, ".* is not a course key"):
|
||||
call_command('reindex_course', unicode(self._get_lib_key(self.second_lib)))
|
||||
|
||||
with self.assertRaises(SystemExit), nostderr():
|
||||
with self.assertRaisesRegexp(SearchIndexingError, ".* is not a course key"):
|
||||
call_command(
|
||||
'reindex_course',
|
||||
unicode(self.second_course.id),
|
||||
unicode(self._get_lib_key(self.first_lib))
|
||||
)
|
||||
|
||||
def test_given_id_list_indexes_courses(self):
|
||||
""" Test that reindexes courses when given single course key or a list of course keys """
|
||||
with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \
|
||||
mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)):
|
||||
call_command('reindex_course', unicode(self.first_course.id))
|
||||
self.assertEqual(patched_index.mock_calls, self._build_calls(self.first_course))
|
||||
patched_index.reset_mock()
|
||||
|
||||
call_command('reindex_course', unicode(self.second_course.id))
|
||||
self.assertEqual(patched_index.mock_calls, self._build_calls(self.second_course))
|
||||
patched_index.reset_mock()
|
||||
|
||||
call_command(
|
||||
'reindex_course',
|
||||
unicode(self.first_course.id),
|
||||
unicode(self.second_course.id)
|
||||
)
|
||||
expected_calls = self._build_calls(self.first_course, self.second_course)
|
||||
self.assertEqual(patched_index.mock_calls, expected_calls)
|
||||
|
||||
def test_given_all_key_prompts_and_reindexes_all_courses(self):
|
||||
""" Test that reindexes all courses when --all key is given and confirmed """
|
||||
with mock.patch(self.YESNO_PATCH_LOCATION) as patched_yes_no:
|
||||
patched_yes_no.return_value = True
|
||||
with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \
|
||||
mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)):
|
||||
call_command('reindex_course', all=True)
|
||||
|
||||
patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no')
|
||||
expected_calls = self._build_calls(self.first_course, self.second_course)
|
||||
self.assertItemsEqual(patched_index.mock_calls, expected_calls)
|
||||
|
||||
def test_given_all_key_prompts_and_reindexes_all_courses_cancelled(self):
|
||||
""" Test that does not reindex anything when --all key is given and cancelled """
|
||||
with mock.patch(self.YESNO_PATCH_LOCATION) as patched_yes_no:
|
||||
patched_yes_no.return_value = False
|
||||
with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \
|
||||
mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)):
|
||||
call_command('reindex_course', all=True)
|
||||
|
||||
patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no')
|
||||
patched_index.assert_not_called()
|
||||
|
||||
def test_fail_fast_if_reindex_fails(self):
|
||||
""" Test that fails on first reindexing exception """
|
||||
with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index:
|
||||
patched_index.side_effect = SearchIndexingError("message", [])
|
||||
|
||||
with self.assertRaises(SearchIndexingError):
|
||||
call_command('reindex_course', unicode(self.second_course.id))
|
||||
@@ -395,6 +395,9 @@ FEATURES = {
|
||||
# Course discovery feature
|
||||
'ENABLE_COURSE_DISCOVERY': False,
|
||||
|
||||
# Setting for overriding default filtering facets for Course discovery
|
||||
# COURSE_DISCOVERY_FILTERS = ["org", "language", "modes"]
|
||||
|
||||
# Software secure fake page feature flag
|
||||
'ENABLE_SOFTWARE_SECURE_FAKE': False,
|
||||
|
||||
|
||||
@@ -138,7 +138,7 @@ FEATURES['LICENSING'] = True
|
||||
|
||||
|
||||
########################## Courseware Search #######################
|
||||
FEATURES['ENABLE_COURSEWARE_SEARCH'] = False
|
||||
FEATURES['ENABLE_COURSEWARE_SEARCH'] = True
|
||||
SEARCH_ENGINE = "search.elastic.ElasticSearchEngine"
|
||||
|
||||
|
||||
@@ -167,7 +167,9 @@ COURSE_DISCOVERY_MEANINGS = {
|
||||
'language': LANGUAGE_MAP,
|
||||
}
|
||||
|
||||
FEATURES['ENABLE_COURSE_DISCOVERY'] = False
|
||||
FEATURES['ENABLE_COURSE_DISCOVERY'] = True
|
||||
# Setting for overriding default filtering facets for Course discovery
|
||||
# COURSE_DISCOVERY_FILTERS = ["org", "language", "modes"]
|
||||
FEATURES['COURSES_ARE_BROWSEABLE'] = True
|
||||
HOMEPAGE_COURSE_MAX = 9
|
||||
|
||||
|
||||
@@ -30,6 +30,9 @@ EXPECTED_CELERY_COMMAND = (
|
||||
EXPECTED_RUN_SERVER_COMMAND = (
|
||||
"python manage.py {system} --settings={settings} runserver --traceback --pythonpath=. 0.0.0.0:{port}"
|
||||
)
|
||||
EXPECTED_INDEX_COURSE_COMMAND = (
|
||||
"python manage.py {system} --settings={settings} reindex_course --setup"
|
||||
)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@@ -83,13 +86,27 @@ class TestPaverServerTasks(PaverTestCase):
|
||||
Test the "devstack" task.
|
||||
"""
|
||||
options = server_options.copy()
|
||||
is_optimized = options.get("optimized", False)
|
||||
expected_settings = "devstack_optimized" if is_optimized else options.get("settings", "devstack")
|
||||
|
||||
# First test with LMS
|
||||
options["system"] = "lms"
|
||||
options["expected_messages"] = [
|
||||
EXPECTED_INDEX_COURSE_COMMAND.format(
|
||||
system="cms",
|
||||
settings=expected_settings,
|
||||
)
|
||||
]
|
||||
self.verify_server_task("devstack", options, contracts_default=True)
|
||||
|
||||
# Then test with Studio
|
||||
options["system"] = "cms"
|
||||
options["expected_messages"] = [
|
||||
EXPECTED_INDEX_COURSE_COMMAND.format(
|
||||
system="cms",
|
||||
settings=expected_settings,
|
||||
)
|
||||
]
|
||||
self.verify_server_task("devstack", options, contracts_default=True)
|
||||
|
||||
@ddt.data(
|
||||
@@ -196,7 +213,7 @@ class TestPaverServerTasks(PaverTestCase):
|
||||
call_task("pavelib.servers.devstack", args=args)
|
||||
else:
|
||||
call_task("pavelib.servers.{task_name}".format(task_name=task_name), options=options)
|
||||
expected_messages = []
|
||||
expected_messages = options.get("expected_messages", [])
|
||||
expected_settings = settings if settings else "devstack"
|
||||
expected_asset_settings = asset_settings if asset_settings else expected_settings
|
||||
if is_optimized:
|
||||
|
||||
@@ -135,6 +135,7 @@ def devstack(args):
|
||||
if args.optimized:
|
||||
settings = OPTIMIZED_SETTINGS
|
||||
asset_settings = OPTIMIZED_ASSETS_SETTINGS
|
||||
sh(django_cmd('cms', settings, 'reindex_course', '--setup'))
|
||||
run_server(
|
||||
args.system[0],
|
||||
fast=args.fast,
|
||||
|
||||
Reference in New Issue
Block a user