From a2dbd403c8a7981595c93f36ef9c3f28b512c50f Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Wed, 29 Jul 2015 11:43:48 +0200 Subject: [PATCH 01/10] add a null signal handler to BulkOperationMixin, remove conditionals --- .../xmodule/xmodule/modulestore/__init__.py | 66 ++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 3017f99d19..8eb64ad2e4 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -154,6 +154,17 @@ class ActiveBulkThread(threading.local): self.records = defaultdict(bulk_ops_record_type) +class NullSignalHandler(object): + """ + A null handler that does nothing + """ + def send(self, *args, **kwargs): + """ + No-op + """ + pass + + class BulkOperationsMixin(object): """ This implements the :meth:`bulk_operations` modulestore semantics which handles nested invocations @@ -169,6 +180,23 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) + self._signal_handler = None + + @property + def signal_handler(self): + """ + Return a signal handler, defaults to a null handler that does nothing. + """ + if not self._signal_handler: + self._signal_handler = NullSignalHandler() + return self._signal_handler + + @signal_handler.setter + def signal_handler(self, value): + """ + Set the signal handler + """ + self._signal_handler = value @contextmanager def bulk_operations(self, course_id, emit_signals=True): @@ -296,18 +324,16 @@ class BulkOperationsMixin(object): """ Sends out the signal that items have been published from within this course. """ - signal_handler = getattr(self, 'signal_handler', None) - if signal_handler and bulk_ops_record.has_publish_item: - signal_handler.send("course_published", course_key=course_id) + if bulk_ops_record.has_publish_item: + self.signal_handler.send("course_published", course_key=course_id) bulk_ops_record.has_publish_item = False def send_bulk_library_updated_signal(self, bulk_ops_record, library_id): """ Sends out the signal that library have been updated. """ - signal_handler = getattr(self, 'signal_handler', None) - if signal_handler and bulk_ops_record.has_library_updated_item: - signal_handler.send("library_updated", library_key=library_id) + if bulk_ops_record.has_library_updated_item: + self.signal_handler.send("library_updated", library_key=library_id) bulk_ops_record.has_library_updated_item = False @@ -1338,13 +1364,11 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): Arguments: course_key - course_key to which the signal applies """ - signal_handler = getattr(self, 'signal_handler', None) - if signal_handler: - bulk_record = self._get_bulk_ops_record(course_key) if isinstance(self, BulkOperationsMixin) else None - if bulk_record and bulk_record.active: - bulk_record.has_publish_item = True - else: - signal_handler.send("course_published", course_key=course_key) + bulk_record = self._get_bulk_ops_record(course_key) if isinstance(self, BulkOperationsMixin) else None + if bulk_record and bulk_record.active: + bulk_record.has_publish_item = True + else: + self.signal_handler.send("course_published", course_key=course_key) def _flag_library_updated_event(self, library_key): """ @@ -1355,21 +1379,17 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): Arguments: library_key - library_key to which the signal applies """ - signal_handler = getattr(self, 'signal_handler', None) - if signal_handler: - bulk_record = self._get_bulk_ops_record(library_key) if isinstance(self, BulkOperationsMixin) else None - if bulk_record and bulk_record.active: - bulk_record.has_library_updated_item = True - else: - signal_handler.send("library_updated", library_key=library_key) + bulk_record = self._get_bulk_ops_record(library_key) if isinstance(self, BulkOperationsMixin) else None + if bulk_record and bulk_record.active: + bulk_record.has_library_updated_item = True + else: + self.signal_handler.send("library_updated", library_key=library_key) def _emit_course_deleted_signal(self, course_key): """ Helper method used to emit the course_deleted signal. """ - signal_handler = getattr(self, 'signal_handler', None) - if signal_handler: - signal_handler.send("course_deleted", course_key=course_key) + self.signal_handler.send("course_deleted", course_key=course_key) def only_xmodules(identifier, entry_points): From 74d7b2a7d6bce90d15d494aff1915327712c21e9 Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Wed, 29 Jul 2015 13:12:21 +0200 Subject: [PATCH 02/10] a different take --- .../xmodule/xmodule/modulestore/__init__.py | 30 ++----------------- .../lib/xmodule/xmodule/modulestore/django.py | 11 +++++++ .../xmodule/xmodule/modulestore/mongo/base.py | 3 +- .../xmodule/modulestore/split_mongo/split.py | 4 ++- 4 files changed, 18 insertions(+), 30 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 8eb64ad2e4..f96edbc58e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -23,6 +23,7 @@ from xblock.plugin import default_select from .exceptions import InvalidLocationError, InsufficientSpecificationError from xmodule.errortracker import make_error_tracker from xmodule.assetstore import AssetMetadata +from xmodule.modulestore.django import NullSignalHandler from opaque_keys.edx.keys import CourseKey, UsageKey, AssetKey from opaque_keys.edx.locations import Location # For import backwards compatibility from xblock.runtime import Mixologist @@ -154,17 +155,6 @@ class ActiveBulkThread(threading.local): self.records = defaultdict(bulk_ops_record_type) -class NullSignalHandler(object): - """ - A null handler that does nothing - """ - def send(self, *args, **kwargs): - """ - No-op - """ - pass - - class BulkOperationsMixin(object): """ This implements the :meth:`bulk_operations` modulestore semantics which handles nested invocations @@ -180,23 +170,7 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) - self._signal_handler = None - - @property - def signal_handler(self): - """ - Return a signal handler, defaults to a null handler that does nothing. - """ - if not self._signal_handler: - self._signal_handler = NullSignalHandler() - return self._signal_handler - - @signal_handler.setter - def signal_handler(self, value): - """ - Set the signal handler - """ - self._signal_handler = value + self.signal_handler = NullSignalHandler() @contextmanager def bulk_operations(self, course_id, emit_signals=True): diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 8b91916b45..d48d573d25 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -110,6 +110,17 @@ class SignalHandler(object): log.info('Sent %s signal to %s with kwargs %s. Response was: %s', signal_name, receiver, kwargs, response) +class NullSignalHandler(object): + """ + A null handler that does nothing + """ + def send(self, *args, **kwargs): + """ + No-op + """ + pass + + def load_function(path): """ Load a function by name. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 898db379c0..f3724628b1 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -49,6 +49,7 @@ from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore from xmodule.modulestore.xml import CourseLocationManager +from xmodule.modulestore.django import NullSignalHandler from xmodule.services import SettingsService log = logging.getLogger(__name__) @@ -543,7 +544,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo i18n_service=None, fs_service=None, user_service=None, - signal_handler=None, + signal_handler=NullSignalHandler(), retry_wait_time=0.1, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 3a829212c0..2adbd140c6 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -85,6 +85,7 @@ from xmodule.error_module import ErrorDescriptor from collections import defaultdict from types import NoneType from xmodule.assetstore import AssetMetadata +from xmodule.modulestore.django import NullSignalHandler log = logging.getLogger(__name__) @@ -640,7 +641,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): default_class=None, error_tracker=null_error_tracker, i18n_service=None, fs_service=None, user_service=None, - services=None, signal_handler=None, **kwargs): + services=None, signal_handler=NullSignalHandler(), + **kwargs): """ :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ From dd457c2f4925ffb813070f8bd599bd20f0f7a27b Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Wed, 29 Jul 2015 18:13:19 +0200 Subject: [PATCH 03/10] Revert "a different take" This reverts commit 74d7b2a7d6bce90d15d494aff1915327712c21e9. --- .../xmodule/xmodule/modulestore/__init__.py | 30 +++++++++++++++++-- .../lib/xmodule/xmodule/modulestore/django.py | 11 ------- .../xmodule/xmodule/modulestore/mongo/base.py | 3 +- .../xmodule/modulestore/split_mongo/split.py | 4 +-- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index f96edbc58e..8eb64ad2e4 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -23,7 +23,6 @@ from xblock.plugin import default_select from .exceptions import InvalidLocationError, InsufficientSpecificationError from xmodule.errortracker import make_error_tracker from xmodule.assetstore import AssetMetadata -from xmodule.modulestore.django import NullSignalHandler from opaque_keys.edx.keys import CourseKey, UsageKey, AssetKey from opaque_keys.edx.locations import Location # For import backwards compatibility from xblock.runtime import Mixologist @@ -155,6 +154,17 @@ class ActiveBulkThread(threading.local): self.records = defaultdict(bulk_ops_record_type) +class NullSignalHandler(object): + """ + A null handler that does nothing + """ + def send(self, *args, **kwargs): + """ + No-op + """ + pass + + class BulkOperationsMixin(object): """ This implements the :meth:`bulk_operations` modulestore semantics which handles nested invocations @@ -170,7 +180,23 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) - self.signal_handler = NullSignalHandler() + self._signal_handler = None + + @property + def signal_handler(self): + """ + Return a signal handler, defaults to a null handler that does nothing. + """ + if not self._signal_handler: + self._signal_handler = NullSignalHandler() + return self._signal_handler + + @signal_handler.setter + def signal_handler(self, value): + """ + Set the signal handler + """ + self._signal_handler = value @contextmanager def bulk_operations(self, course_id, emit_signals=True): diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index d48d573d25..8b91916b45 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -110,17 +110,6 @@ class SignalHandler(object): log.info('Sent %s signal to %s with kwargs %s. Response was: %s', signal_name, receiver, kwargs, response) -class NullSignalHandler(object): - """ - A null handler that does nothing - """ - def send(self, *args, **kwargs): - """ - No-op - """ - pass - - def load_function(path): """ Load a function by name. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index f3724628b1..898db379c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -49,7 +49,6 @@ from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore from xmodule.modulestore.xml import CourseLocationManager -from xmodule.modulestore.django import NullSignalHandler from xmodule.services import SettingsService log = logging.getLogger(__name__) @@ -544,7 +543,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo i18n_service=None, fs_service=None, user_service=None, - signal_handler=NullSignalHandler(), + signal_handler=None, retry_wait_time=0.1, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 2adbd140c6..3a829212c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -85,7 +85,6 @@ from xmodule.error_module import ErrorDescriptor from collections import defaultdict from types import NoneType from xmodule.assetstore import AssetMetadata -from xmodule.modulestore.django import NullSignalHandler log = logging.getLogger(__name__) @@ -641,8 +640,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): default_class=None, error_tracker=null_error_tracker, i18n_service=None, fs_service=None, user_service=None, - services=None, signal_handler=NullSignalHandler(), - **kwargs): + services=None, signal_handler=None, **kwargs): """ :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ From ab923ea250f9daa320b0df440c4c498714d1c337 Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Wed, 29 Jul 2015 19:06:52 +0200 Subject: [PATCH 04/10] use a universal null handler --- .../xmodule/xmodule/modulestore/__init__.py | 28 ------------------- .../lib/xmodule/xmodule/modulestore/django.py | 3 +- .../lib/xmodule/xmodule/modulestore/mixed.py | 3 +- .../xmodule/xmodule/modulestore/mongo/base.py | 3 +- .../xmodule/modulestore/split_mongo/split.py | 3 +- .../xmodule/modulestore/tests/utils.py | 3 +- common/lib/xmodule/xmodule/modulestore/xml.py | 3 +- .../lib/xmodule/xmodule/util/null_handler.py | 9 ++++++ 8 files changed, 21 insertions(+), 34 deletions(-) create mode 100644 common/lib/xmodule/xmodule/util/null_handler.py diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 8eb64ad2e4..f44b48f85a 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -154,17 +154,6 @@ class ActiveBulkThread(threading.local): self.records = defaultdict(bulk_ops_record_type) -class NullSignalHandler(object): - """ - A null handler that does nothing - """ - def send(self, *args, **kwargs): - """ - No-op - """ - pass - - class BulkOperationsMixin(object): """ This implements the :meth:`bulk_operations` modulestore semantics which handles nested invocations @@ -180,23 +169,6 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) - self._signal_handler = None - - @property - def signal_handler(self): - """ - Return a signal handler, defaults to a null handler that does nothing. - """ - if not self._signal_handler: - self._signal_handler = NullSignalHandler() - return self._signal_handler - - @signal_handler.setter - def signal_handler(self, value): - """ - Set the signal handler - """ - self._signal_handler = value @contextmanager def bulk_operations(self, course_id, emit_signals=True): diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 8b91916b45..cedf7a0e5c 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -26,6 +26,7 @@ from xmodule.contentstore.django import contentstore from xmodule.modulestore.draft_and_published import BranchSettingMixin from xmodule.modulestore.mixed import MixedModuleStore from xmodule.util.django import get_current_request_hostname +from xmodule.util.null_handler import NullHandler import xblock.reference.plugins @@ -129,7 +130,7 @@ def create_modulestore_instance( i18n_service=None, fs_service=None, user_service=None, - signal_handler=None, + signal_handler=NullHandler(), ): """ This will return a new instance of a modulestore given an engine and options diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index a49c7dbd9a..8b96687d33 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -16,6 +16,7 @@ from opaque_keys.edx.keys import CourseKey, AssetKey from opaque_keys.edx.locator import LibraryLocator from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.assetstore import AssetMetadata +from xmodule.util.null_handler import NullHandler from . import ModuleStoreWriteBase from . import ModuleStoreEnum @@ -108,7 +109,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): fs_service=None, user_service=None, create_modulestore_instance=None, - signal_handler=None, + signal_handler=NullHandler(), **kwargs ): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 898db379c0..94fba7671a 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -50,6 +50,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseErr from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore from xmodule.modulestore.xml import CourseLocationManager from xmodule.services import SettingsService +from xmodule.util.null_handler import NullHandler log = logging.getLogger(__name__) @@ -543,7 +544,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo i18n_service=None, fs_service=None, user_service=None, - signal_handler=None, + signal_handler=NullHandler(), retry_wait_time=0.1, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 3a829212c0..b671d0e9db 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -67,6 +67,7 @@ from bson.objectid import ObjectId from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.errortracker import null_error_tracker +from xmodule.util.null_handler import NullHandler from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) @@ -640,7 +641,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): default_class=None, error_tracker=null_error_tracker, i18n_service=None, fs_service=None, user_service=None, - services=None, signal_handler=None, **kwargs): + services=None, signal_handler=NullHandler(), **kwargs): """ :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index aa8832ba10..482adb74e1 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -6,6 +6,7 @@ from opaque_keys.edx.keys import UsageKey from unittest import TestCase from xblock.fields import XBlockMixin from xmodule.x_module import XModuleMixin +from xmodule.util.null_handler import NullHandler from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoMixin @@ -36,7 +37,7 @@ def create_modulestore_instance( i18n_service=None, fs_service=None, user_service=None, - signal_handler=None, + signal_handler=NullHandler(), ): """ This will return a new instance of a modulestore given an engine and options diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 6dd2fec98d..d63db5bd9c 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -26,6 +26,7 @@ from xmodule.x_module import ( from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase, LIBRARY_ROOT, COURSE_ROOT from xmodule.tabs import CourseTabList +from xmodule.util.null_handler import NullHandler from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locator import CourseLocator, LibraryLocator @@ -335,7 +336,7 @@ class XMLModuleStore(ModuleStoreReadBase): def __init__( self, data_dir, default_class=None, source_dirs=None, course_ids=None, load_error_modules=True, i18n_service=None, fs_service=None, user_service=None, - signal_handler=None, target_course_id=None, **kwargs # pylint: disable=unused-argument + signal_handler=NullHandler(), target_course_id=None, **kwargs # pylint: disable=unused-argument ): """ Initialize an XMLModuleStore from data_dir diff --git a/common/lib/xmodule/xmodule/util/null_handler.py b/common/lib/xmodule/xmodule/util/null_handler.py new file mode 100644 index 0000000000..b5e36f6901 --- /dev/null +++ b/common/lib/xmodule/xmodule/util/null_handler.py @@ -0,0 +1,9 @@ +class NullHandler(object): + """ + Responds to an any method call. + """ + def __getattr__(self, name): + def method(*args, **kwargs): + pass + return method + From 7a923cf520fdd0f206d2c8d978606eac0dca7b6d Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Wed, 29 Jul 2015 20:30:39 +0200 Subject: [PATCH 05/10] fix errors --- common/lib/xmodule/xmodule/modulestore/__init__.py | 1 + common/lib/xmodule/xmodule/util/null_handler.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index f44b48f85a..a70a9c42f9 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -169,6 +169,7 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) + self.signal_handler = None @contextmanager def bulk_operations(self, course_id, emit_signals=True): diff --git a/common/lib/xmodule/xmodule/util/null_handler.py b/common/lib/xmodule/xmodule/util/null_handler.py index b5e36f6901..a628f43fdc 100644 --- a/common/lib/xmodule/xmodule/util/null_handler.py +++ b/common/lib/xmodule/xmodule/util/null_handler.py @@ -1,9 +1,10 @@ +# pylint: disable=missing-docstring + class NullHandler(object): """ Responds to an any method call. """ def __getattr__(self, name): - def method(*args, **kwargs): + def method(*args, **kwargs): # pylint: disable=unused-argument pass return method - From a62ef9ffa9aaa7cde2818f9fd972ad38c4227630 Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Wed, 29 Jul 2015 21:24:38 +0200 Subject: [PATCH 06/10] quality --- common/lib/xmodule/xmodule/util/null_handler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/util/null_handler.py b/common/lib/xmodule/xmodule/util/null_handler.py index a628f43fdc..db4f989d11 100644 --- a/common/lib/xmodule/xmodule/util/null_handler.py +++ b/common/lib/xmodule/xmodule/util/null_handler.py @@ -1,10 +1,11 @@ # pylint: disable=missing-docstring + class NullHandler(object): """ Responds to an any method call. """ def __getattr__(self, name): - def method(*args, **kwargs): # pylint: disable=unused-argument + def method(*args, **kwargs): # pylint: disable=unused-argument pass return method From 73f163091f9c3b4fb0e88b9272a1a4bcdde70c38 Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Thu, 30 Jul 2015 00:42:49 +0200 Subject: [PATCH 07/10] Initial idea --- .../xmodule/xmodule/modulestore/__init__.py | 29 ++++++++++++++++++- .../lib/xmodule/xmodule/modulestore/django.py | 3 +- .../lib/xmodule/xmodule/modulestore/mixed.py | 3 +- .../xmodule/xmodule/modulestore/mongo/base.py | 3 +- .../xmodule/modulestore/split_mongo/split.py | 3 +- .../xmodule/modulestore/tests/utils.py | 3 +- common/lib/xmodule/xmodule/modulestore/xml.py | 3 +- 7 files changed, 34 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index a70a9c42f9..8eb64ad2e4 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -154,6 +154,17 @@ class ActiveBulkThread(threading.local): self.records = defaultdict(bulk_ops_record_type) +class NullSignalHandler(object): + """ + A null handler that does nothing + """ + def send(self, *args, **kwargs): + """ + No-op + """ + pass + + class BulkOperationsMixin(object): """ This implements the :meth:`bulk_operations` modulestore semantics which handles nested invocations @@ -169,7 +180,23 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) - self.signal_handler = None + self._signal_handler = None + + @property + def signal_handler(self): + """ + Return a signal handler, defaults to a null handler that does nothing. + """ + if not self._signal_handler: + self._signal_handler = NullSignalHandler() + return self._signal_handler + + @signal_handler.setter + def signal_handler(self, value): + """ + Set the signal handler + """ + self._signal_handler = value @contextmanager def bulk_operations(self, course_id, emit_signals=True): diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index cedf7a0e5c..8b91916b45 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -26,7 +26,6 @@ from xmodule.contentstore.django import contentstore from xmodule.modulestore.draft_and_published import BranchSettingMixin from xmodule.modulestore.mixed import MixedModuleStore from xmodule.util.django import get_current_request_hostname -from xmodule.util.null_handler import NullHandler import xblock.reference.plugins @@ -130,7 +129,7 @@ def create_modulestore_instance( i18n_service=None, fs_service=None, user_service=None, - signal_handler=NullHandler(), + signal_handler=None, ): """ This will return a new instance of a modulestore given an engine and options diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 8b96687d33..a49c7dbd9a 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -16,7 +16,6 @@ from opaque_keys.edx.keys import CourseKey, AssetKey from opaque_keys.edx.locator import LibraryLocator from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.assetstore import AssetMetadata -from xmodule.util.null_handler import NullHandler from . import ModuleStoreWriteBase from . import ModuleStoreEnum @@ -109,7 +108,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): fs_service=None, user_service=None, create_modulestore_instance=None, - signal_handler=NullHandler(), + signal_handler=None, **kwargs ): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 94fba7671a..898db379c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -50,7 +50,6 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseErr from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore from xmodule.modulestore.xml import CourseLocationManager from xmodule.services import SettingsService -from xmodule.util.null_handler import NullHandler log = logging.getLogger(__name__) @@ -544,7 +543,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo i18n_service=None, fs_service=None, user_service=None, - signal_handler=NullHandler(), + signal_handler=None, retry_wait_time=0.1, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index b671d0e9db..3a829212c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -67,7 +67,6 @@ from bson.objectid import ObjectId from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.errortracker import null_error_tracker -from xmodule.util.null_handler import NullHandler from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) @@ -641,7 +640,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): default_class=None, error_tracker=null_error_tracker, i18n_service=None, fs_service=None, user_service=None, - services=None, signal_handler=NullHandler(), **kwargs): + services=None, signal_handler=None, **kwargs): """ :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index 482adb74e1..aa8832ba10 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -6,7 +6,6 @@ from opaque_keys.edx.keys import UsageKey from unittest import TestCase from xblock.fields import XBlockMixin from xmodule.x_module import XModuleMixin -from xmodule.util.null_handler import NullHandler from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoMixin @@ -37,7 +36,7 @@ def create_modulestore_instance( i18n_service=None, fs_service=None, user_service=None, - signal_handler=NullHandler(), + signal_handler=None, ): """ This will return a new instance of a modulestore given an engine and options diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index d63db5bd9c..6dd2fec98d 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -26,7 +26,6 @@ from xmodule.x_module import ( from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase, LIBRARY_ROOT, COURSE_ROOT from xmodule.tabs import CourseTabList -from xmodule.util.null_handler import NullHandler from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locator import CourseLocator, LibraryLocator @@ -336,7 +335,7 @@ class XMLModuleStore(ModuleStoreReadBase): def __init__( self, data_dir, default_class=None, source_dirs=None, course_ids=None, load_error_modules=True, i18n_service=None, fs_service=None, user_service=None, - signal_handler=NullHandler(), target_course_id=None, **kwargs # pylint: disable=unused-argument + signal_handler=None, target_course_id=None, **kwargs # pylint: disable=unused-argument ): """ Initialize an XMLModuleStore from data_dir From a7f2b025a9de06ec6ca917e7cd60dafdc18041da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davorin=20=C5=A0ego?= Date: Thu, 30 Jul 2015 16:00:31 +0200 Subject: [PATCH 08/10] Delete null_handler.py --- common/lib/xmodule/xmodule/util/null_handler.py | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/util/null_handler.py diff --git a/common/lib/xmodule/xmodule/util/null_handler.py b/common/lib/xmodule/xmodule/util/null_handler.py deleted file mode 100644 index db4f989d11..0000000000 --- a/common/lib/xmodule/xmodule/util/null_handler.py +++ /dev/null @@ -1,11 +0,0 @@ -# pylint: disable=missing-docstring - - -class NullHandler(object): - """ - Responds to an any method call. - """ - def __getattr__(self, name): - def method(*args, **kwargs): # pylint: disable=unused-argument - pass - return method From d06e16d9532793304ded940c46a58be33c568bd5 Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Thu, 6 Aug 2015 22:06:56 +0200 Subject: [PATCH 09/10] Initialize signal handler, so we can remove getattr --- .../xmodule/xmodule/modulestore/__init__.py | 57 ++++++------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 8eb64ad2e4..fc9a1daaa6 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -154,17 +154,6 @@ class ActiveBulkThread(threading.local): self.records = defaultdict(bulk_ops_record_type) -class NullSignalHandler(object): - """ - A null handler that does nothing - """ - def send(self, *args, **kwargs): - """ - No-op - """ - pass - - class BulkOperationsMixin(object): """ This implements the :meth:`bulk_operations` modulestore semantics which handles nested invocations @@ -180,23 +169,8 @@ class BulkOperationsMixin(object): def __init__(self, *args, **kwargs): super(BulkOperationsMixin, self).__init__(*args, **kwargs) self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) - self._signal_handler = None + self.signal_handler = None - @property - def signal_handler(self): - """ - Return a signal handler, defaults to a null handler that does nothing. - """ - if not self._signal_handler: - self._signal_handler = NullSignalHandler() - return self._signal_handler - - @signal_handler.setter - def signal_handler(self, value): - """ - Set the signal handler - """ - self._signal_handler = value @contextmanager def bulk_operations(self, course_id, emit_signals=True): @@ -324,7 +298,7 @@ class BulkOperationsMixin(object): """ Sends out the signal that items have been published from within this course. """ - if bulk_ops_record.has_publish_item: + if self.signal_handler and bulk_ops_record.has_publish_item: self.signal_handler.send("course_published", course_key=course_id) bulk_ops_record.has_publish_item = False @@ -332,7 +306,7 @@ class BulkOperationsMixin(object): """ Sends out the signal that library have been updated. """ - if bulk_ops_record.has_library_updated_item: + if self.signal_handler and bulk_ops_record.has_library_updated_item: self.signal_handler.send("library_updated", library_key=library_id) bulk_ops_record.has_library_updated_item = False @@ -1364,11 +1338,12 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): Arguments: course_key - course_key to which the signal applies """ - bulk_record = self._get_bulk_ops_record(course_key) if isinstance(self, BulkOperationsMixin) else None - if bulk_record and bulk_record.active: - bulk_record.has_publish_item = True - else: - self.signal_handler.send("course_published", course_key=course_key) + if self.signal_handler: + bulk_record = self._get_bulk_ops_record(course_key) if isinstance(self, BulkOperationsMixin) else None + if bulk_record and bulk_record.active: + bulk_record.has_publish_item = True + else: + self.signal_handler.send("course_published", course_key=course_key) def _flag_library_updated_event(self, library_key): """ @@ -1379,17 +1354,19 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): Arguments: library_key - library_key to which the signal applies """ - bulk_record = self._get_bulk_ops_record(library_key) if isinstance(self, BulkOperationsMixin) else None - if bulk_record and bulk_record.active: - bulk_record.has_library_updated_item = True - else: - self.signal_handler.send("library_updated", library_key=library_key) + if self.signal_handler: + bulk_record = self._get_bulk_ops_record(library_key) if isinstance(self, BulkOperationsMixin) else None + if bulk_record and bulk_record.active: + bulk_record.has_library_updated_item = True + else: + self.signal_handler.send("library_updated", library_key=library_key) def _emit_course_deleted_signal(self, course_key): """ Helper method used to emit the course_deleted signal. """ - self.signal_handler.send("course_deleted", course_key=course_key) + if self.signal_handler: + self.signal_handler.send("course_deleted", course_key=course_key) def only_xmodules(identifier, entry_points): From 0d4f602e33dcb12008a74571d71dbe91d7cc83bc Mon Sep 17 00:00:00 2001 From: Davorin Sego Date: Thu, 6 Aug 2015 22:35:44 +0200 Subject: [PATCH 10/10] pep8 --- common/lib/xmodule/xmodule/modulestore/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index fc9a1daaa6..42aec05184 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -171,7 +171,6 @@ class BulkOperationsMixin(object): self._active_bulk_ops = ActiveBulkThread(self._bulk_ops_record_type) self.signal_handler = None - @contextmanager def bulk_operations(self, course_id, emit_signals=True): """