From a218e66d2eca9c174f7c68bcfc9f6d0eb51137bd Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 10 Jul 2014 19:26:03 -0400 Subject: [PATCH 1/4] auto-migrate old mongo to draft modulestore --- lms/envs/modulestore_settings.py | 6 ++ lms/envs/test_modulestore_settings.py | 133 ++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 lms/envs/test_modulestore_settings.py diff --git a/lms/envs/modulestore_settings.py b/lms/envs/modulestore_settings.py index 9ca8d1060c..9eaad4ffc2 100644 --- a/lms/envs/modulestore_settings.py +++ b/lms/envs/modulestore_settings.py @@ -14,11 +14,17 @@ def convert_module_store_setting_if_needed(module_store_setting): """ new_store_list = [] for store_name, store_settings in old_stores.iteritems(): + store_settings['NAME'] = store_name if store_name == 'default': new_store_list.insert(0, store_settings) else: new_store_list.append(store_settings) + + # migrate request for the old 'direct' Mongo store to the Draft store + if store_settings['ENGINE'] == 'xmodule.modulestore.mongo.MongoModuleStore': + store_settings['ENGINE'] = 'xmodule.modulestore.mongo.draft.DraftModuleStore' + return new_store_list if module_store_setting is None: diff --git a/lms/envs/test_modulestore_settings.py b/lms/envs/test_modulestore_settings.py new file mode 100644 index 0000000000..3f1e1ba8e7 --- /dev/null +++ b/lms/envs/test_modulestore_settings.py @@ -0,0 +1,133 @@ +""" +Tests for testing the modulestore settings migration code. +""" +import copy +from django.test import TestCase +from lms.envs.modulestore_settings import convert_module_store_setting_if_needed + + +class ModuleStoreSettingsMigration(TestCase): + """ + Tests for the migration code for the module store settings + """ + + OLD_CONFIG = { + "default": { + "ENGINE": "xmodule.modulestore.xml.XMLModuleStore", + "OPTIONS": { + "data_dir": "directory", + "default_class": "xmodule.hidden_module.HiddenDescriptor", + }, + "DOC_STORE_CONFIG": {}, + } + } + + OLD_CONFIG_WITH_DIRECT_MONGO = { + "default": { + "ENGINE": "xmodule.modulestore.mongo.MongoModuleStore", + "OPTIONS": { + "collection": "modulestore", + "db": "edxapp", + "default_class": "xmodule.hidden_module.HiddenDescriptor", + "fs_root": "/edx/var/edxapp/data", + "host": "localhost", + "password": "password", + "port": 27017, + "render_template": "edxmako.shortcuts.render_to_string", + "user": "edxapp" + }, + "DOC_STORE_CONFIG": {}, + } + } + + OLD_MIXED_CONFIG_WITH_DICT = { + "default": { + "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore", + "OPTIONS": { + "mappings": {}, + "reference_type": "Location", + "stores": { + "an_old_mongo_store": { + "DOC_STORE_CONFIG": {}, + "ENGINE": "xmodule.modulestore.mongo.MongoModuleStore", + "OPTIONS": { + "collection": "modulestore", + "db": "test", + "default_class": "xmodule.hidden_module.HiddenDescriptor", + } + }, + "default": { + "ENGINE": "the_default_store", + "OPTIONS": { + "option1": "value1", + "option2": "value2" + }, + "DOC_STORE_CONFIG": {} + }, + "xml": { + "ENGINE": "xmodule.modulestore.xml.XMLModuleStore", + "OPTIONS": { + "data_dir": "directory", + "default_class": "xmodule.hidden_module.HiddenDescriptor" + }, + "DOC_STORE_CONFIG": {} + } + } + } + } + } + + def _get_mixed_stores(self, mixed_setting): + """ + Helper for accessing stores in a configuration setting for the Mixed modulestore + """ + return mixed_setting["default"]["OPTIONS"]["stores"] + + def assertStoreValuesEqual(self, store_setting1, store_setting2): + """ + Tests whether the fields in the given store_settings are equal + """ + store_fields = ["OPTIONS", "DOC_STORE_CONFIG"] + for field in store_fields: + self.assertEqual(store_setting1[field], store_setting2[field]) + + def assertMigrated(self, old_setting): + """ + Migrates the given setting and checks whether it correctly converted + to an ordered list of stores within Mixed. + """ + # pass a copy of the old setting since the migration modifies the given setting + new_mixed_setting = convert_module_store_setting_if_needed(copy.deepcopy(old_setting)) + + # check whether the configuration is encapsulated within Mixed. + self.assertEqual(new_mixed_setting["default"]["ENGINE"], "xmodule.modulestore.mixed.MixedModuleStore") + + # check whether the stores are in an ordered list + new_stores = self._get_mixed_stores(new_mixed_setting) + self.assertIsInstance(new_stores, list) + + return new_mixed_setting, new_stores[0] + + def test_convert_into_mixed(self): + old_setting = self.OLD_CONFIG + _, new_default_store_setting = self.assertMigrated(old_setting) + self.assertStoreValuesEqual(new_default_store_setting, old_setting["default"]) + self.assertEqual(new_default_store_setting["ENGINE"], old_setting["default"]["ENGINE"]) + + def test_convert_from_old_mongo_to_draft_store(self): + old_setting = self.OLD_CONFIG_WITH_DIRECT_MONGO + _, new_default_store_setting = self.assertMigrated(old_setting) + self.assertStoreValuesEqual(new_default_store_setting, old_setting["default"]) + self.assertEqual(new_default_store_setting["ENGINE"], "xmodule.modulestore.mongo.draft.DraftModuleStore") + + def test_convert_from_dict_to_list(self): + old_mixed_setting = self.OLD_MIXED_CONFIG_WITH_DICT + new_mixed_setting, new_default_store_setting = self.assertMigrated(old_mixed_setting) + self.assertEqual(new_default_store_setting["ENGINE"], "the_default_store") + + # compare each store configured in mixed + old_stores = self._get_mixed_stores(self.OLD_MIXED_CONFIG_WITH_DICT) + new_stores = self._get_mixed_stores(new_mixed_setting) + self.assertEqual(len(new_stores), len(old_stores)) + for new_store_setting in self._get_mixed_stores(new_mixed_setting): + self.assertStoreValuesEqual(new_store_setting, old_stores[new_store_setting['NAME']]) From 4859eee23bdba97fc0c3cdf1b20685fa85bb21a3 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 11 Jul 2014 16:11:45 -0400 Subject: [PATCH 2/4] Move modulestore django settings migration code down into the modulestore layer so it works for all django-based callers. --- cms/envs/aws.py | 2 +- cms/envs/common.py | 4 ++-- common/lib/xmodule/xmodule/modulestore/django.py | 2 ++ .../lib/xmodule/xmodule/modulestore}/modulestore_settings.py | 0 .../xmodule/modulestore/tests}/test_modulestore_settings.py | 2 +- lms/envs/aws.py | 2 +- lms/envs/common.py | 2 +- 7 files changed, 8 insertions(+), 6 deletions(-) rename {lms/envs => common/lib/xmodule/xmodule/modulestore}/modulestore_settings.py (100%) rename {lms/envs => common/lib/xmodule/xmodule/modulestore/tests}/test_modulestore_settings.py (98%) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 0d4d20b0ce..6b04d1fbf6 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -229,7 +229,7 @@ if AWS_SECRET_ACCESS_KEY == "": AWS_SECRET_ACCESS_KEY = None DATABASES = AUTH_TOKENS['DATABASES'] -MODULESTORE = convert_module_store_setting_if_needed(AUTH_TOKENS['MODULESTORE']) +MODULESTORE = AUTH_TOKENS.get('MODULESTORE', MODULESTORE) CONTENTSTORE = AUTH_TOKENS['CONTENTSTORE'] DOC_STORE_CONFIG = AUTH_TOKENS['DOC_STORE_CONFIG'] # Datadog for events! diff --git a/cms/envs/common.py b/cms/envs/common.py index a5dd3ddad2..a9be299409 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -30,11 +30,11 @@ import sys import lms.envs.common # Although this module itself may not use these imported variables, other dependent modules may. from lms.envs.common import ( - USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, BUGS_EMAIL, DOC_STORE_CONFIG, ALL_LANGUAGES, WIKI_ENABLED, MODULESTORE + USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, BUGS_EMAIL, DOC_STORE_CONFIG, ALL_LANGUAGES, WIKI_ENABLED, MODULESTORE, + update_module_store_settings ) from path import path from warnings import simplefilter -from lms.envs.modulestore_settings import * from lms.lib.xblock.mixin import LmsBlockMixin from dealer.git import git diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 973833f4f8..5107181e6d 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -18,6 +18,7 @@ from xmodule.modulestore.loc_mapper_store import LocMapperStore from xmodule.util.django import get_current_request_hostname import xmodule.modulestore # pylint: disable=unused-import from xmodule.contentstore.django import contentstore +from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed # We may not always have the request_cache module available try: @@ -85,6 +86,7 @@ def modulestore(): """ global _MIXED_MODULESTORE # pylint: disable=global-statement if _MIXED_MODULESTORE is None: + settings.MODULESTORE = convert_module_store_setting_if_needed(settings.MODULESTORE) _MIXED_MODULESTORE = create_modulestore_instance( settings.MODULESTORE['default']['ENGINE'], contentstore(), diff --git a/lms/envs/modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py similarity index 100% rename from lms/envs/modulestore_settings.py rename to common/lib/xmodule/xmodule/modulestore/modulestore_settings.py diff --git a/lms/envs/test_modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py similarity index 98% rename from lms/envs/test_modulestore_settings.py rename to common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py index 3f1e1ba8e7..eb96394c27 100644 --- a/lms/envs/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -3,7 +3,7 @@ Tests for testing the modulestore settings migration code. """ import copy from django.test import TestCase -from lms.envs.modulestore_settings import convert_module_store_setting_if_needed +from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed class ModuleStoreSettingsMigration(TestCase): diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 70d1f19ee5..1306c4dcdf 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -324,7 +324,7 @@ XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE'] # Get the MODULESTORE from auth.json, but if it doesn't exist, # use the one from common.py -MODULESTORE = convert_module_store_setting_if_needed(AUTH_TOKENS.get('MODULESTORE', MODULESTORE)) +MODULESTORE = AUTH_TOKENS.get('MODULESTORE', MODULESTORE) CONTENTSTORE = AUTH_TOKENS.get('CONTENTSTORE', CONTENTSTORE) DOC_STORE_CONFIG = AUTH_TOKENS.get('DOC_STORE_CONFIG', DOC_STORE_CONFIG) MONGODB_LOG = AUTH_TOKENS.get('MONGODB_LOG', {}) diff --git a/lms/envs/common.py b/lms/envs/common.py index eb54f7e58d..d1d764341d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -33,7 +33,7 @@ from path import path from warnings import simplefilter from .discussionsettings import * -from .modulestore_settings import * +from xmodule.modulestore.modulestore_settings import update_module_store_settings from lms.lib.xblock.mixin import LmsBlockMixin From 7503cc247391773745f95d3ec53cfcd8ca8c9326 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 11 Jul 2014 16:58:27 -0400 Subject: [PATCH 3/4] reverting back to migrating the settings in the env files --- cms/envs/aws.py | 3 ++- cms/envs/devstack.py | 6 +++++- common/lib/xmodule/xmodule/modulestore/django.py | 2 -- .../xmodule/modulestore/tests/test_modulestore_settings.py | 2 +- lms/envs/aws.py | 3 ++- lms/envs/devstack.py | 6 +++++- 6 files changed, 15 insertions(+), 7 deletions(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 6b04d1fbf6..63605461c2 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -15,6 +15,7 @@ import os from path import path from dealer.git import git +from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed # SERVICE_VARIANT specifies name of the variant used, which decides what JSON # configuration files are read during startup. @@ -229,7 +230,7 @@ if AWS_SECRET_ACCESS_KEY == "": AWS_SECRET_ACCESS_KEY = None DATABASES = AUTH_TOKENS['DATABASES'] -MODULESTORE = AUTH_TOKENS.get('MODULESTORE', MODULESTORE) +MODULESTORE = convert_module_store_setting_if_needed(AUTH_TOKENS.get('MODULESTORE', MODULESTORE)) CONTENTSTORE = AUTH_TOKENS['CONTENTSTORE'] DOC_STORE_CONFIG = AUTH_TOKENS['DOC_STORE_CONFIG'] # Datadog for events! diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 6dc95c1925..145c1d8480 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -72,8 +72,12 @@ DEBUG_TOOLBAR_CONFIG = { DEBUG_TOOLBAR_MONGO_STACKTRACES = False ############################################################################### -# Lastly, see if the developer has any local overrides. +# See if the developer has any local overrides. try: from .private import * # pylint: disable=F0401 except ImportError: pass + +##################################################################### +# Lastly, run any migrations, if needed. +MODULESTORE = convert_module_store_setting_if_needed(MODULESTORE) diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 5107181e6d..973833f4f8 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -18,7 +18,6 @@ from xmodule.modulestore.loc_mapper_store import LocMapperStore from xmodule.util.django import get_current_request_hostname import xmodule.modulestore # pylint: disable=unused-import from xmodule.contentstore.django import contentstore -from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed # We may not always have the request_cache module available try: @@ -86,7 +85,6 @@ def modulestore(): """ global _MIXED_MODULESTORE # pylint: disable=global-statement if _MIXED_MODULESTORE is None: - settings.MODULESTORE = convert_module_store_setting_if_needed(settings.MODULESTORE) _MIXED_MODULESTORE = create_modulestore_instance( settings.MODULESTORE['default']['ENGINE'], contentstore(), diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py index eb96394c27..a0b63d4691 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -2,7 +2,7 @@ Tests for testing the modulestore settings migration code. """ import copy -from django.test import TestCase +from unittest import TestCase from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 1306c4dcdf..a197cbd480 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -18,6 +18,7 @@ from logsettings import get_logger_config import os from path import path +from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed # SERVICE_VARIANT specifies name of the variant used, which decides what JSON # configuration files are read during startup. @@ -324,7 +325,7 @@ XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE'] # Get the MODULESTORE from auth.json, but if it doesn't exist, # use the one from common.py -MODULESTORE = AUTH_TOKENS.get('MODULESTORE', MODULESTORE) +MODULESTORE = convert_module_store_setting_if_needed(AUTH_TOKENS.get('MODULESTORE', MODULESTORE)) CONTENTSTORE = AUTH_TOKENS.get('CONTENTSTORE', CONTENTSTORE) DOC_STORE_CONFIG = AUTH_TOKENS.get('DOC_STORE_CONFIG', DOC_STORE_CONFIG) MONGODB_LOG = AUTH_TOKENS.get('MONGODB_LOG', {}) diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index a89de22001..330725df2e 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -82,8 +82,12 @@ FEATURES['ENABLE_PAYMENT_FAKE'] = True CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = '/shoppingcart/payment_fake/' ##################################################################### -# Lastly, see if the developer has any local overrides. +# See if the developer has any local overrides. try: from .private import * # pylint: disable=F0401 except ImportError: pass + +##################################################################### +# Lastly, run any migrations, if needed. +MODULESTORE = convert_module_store_setting_if_needed(MODULESTORE) From 23c5febc173c6520fee224342b62b81d357aecca Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 14 Jul 2014 11:57:31 -0400 Subject: [PATCH 4/4] Added deprecation warnings. --- .../xmodule/modulestore/modulestore_settings.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py index 9eaad4ffc2..b304af84eb 100644 --- a/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py @@ -2,6 +2,12 @@ This file contains helper functions for configuring module_store_setting settings and support for backward compatibility with older formats. """ +import warnings + + +# Python 2.7 by default suppresses DeprecationWarnings. Make sure we show these, always. +warnings.simplefilter('once', DeprecationWarning) + def convert_module_store_setting_if_needed(module_store_setting): """ @@ -23,6 +29,7 @@ def convert_module_store_setting_if_needed(module_store_setting): # migrate request for the old 'direct' Mongo store to the Draft store if store_settings['ENGINE'] == 'xmodule.modulestore.mongo.MongoModuleStore': + warnings.warn("MongoModuleStore is deprecated! Please use DraftModuleStore.", DeprecationWarning) store_settings['ENGINE'] = 'xmodule.modulestore.mongo.draft.DraftModuleStore' return new_store_list @@ -31,6 +38,8 @@ def convert_module_store_setting_if_needed(module_store_setting): return None if module_store_setting['default']['ENGINE'] != 'xmodule.modulestore.mixed.MixedModuleStore': + warnings.warn("Direct access to a modulestore is deprecated. Please use MixedModuleStore.", DeprecationWarning) + # convert to using mixed module_store new_module_store_setting = { "default": { @@ -50,6 +59,11 @@ def convert_module_store_setting_if_needed(module_store_setting): module_store_setting = new_module_store_setting elif isinstance(module_store_setting['default']['OPTIONS']['stores'], dict): + warnings.warn( + "Using a dict for the Stores option in the MixedModuleStore is deprecated. Please use a list instead.", + DeprecationWarning + ) + # convert old-style (unordered) dict to (an ordered) list module_store_setting['default']['OPTIONS']['stores'] = convert_old_stores_into_list( module_store_setting['default']['OPTIONS']['stores']