From 86be73f6a237eed01324a39fcc7b866063fcea1e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 11:33:59 -0500 Subject: [PATCH 1/3] Make errors during course loading not break the site, but just hide that course instead --- common/lib/xmodule/xmodule/modulestore/xml.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index b7f0c726e4..3e4cd4e899 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -13,6 +13,7 @@ from importlib import import_module from lxml import etree from path import path +from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import make_error_tracker, exc_info_to_str from xmodule.course_module import CourseDescriptor from xmodule.mako_module import MakoDescriptorSystem @@ -167,8 +168,6 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # Didn't load properly. Fall back on loading as an error # descriptor. This should never error due to formatting. - # Put import here to avoid circular import errors - from xmodule.error_module import ErrorDescriptor msg = "Error loading from xml. " + str(err)[:200] log.warning(msg) @@ -311,7 +310,7 @@ class XMLModuleStore(ModuleStoreBase): log.exception(msg) errorlog.tracker(msg) - if course_descriptor is not None: + if course_descriptor is not None and not isinstance(course_descriptor, ErrorDescriptor): self.courses[course_dir] = course_descriptor self._location_errors[course_descriptor.location] = errorlog self.parent_trackers[course_descriptor.id].make_known(course_descriptor.location) @@ -423,6 +422,10 @@ class XMLModuleStore(ModuleStoreBase): course_descriptor = system.process_xml(etree.tostring(course_data, encoding='unicode')) + # If we fail to load the course, then skip the rest of the loading steps + if isinstance(course_descriptor, ErrorDescriptor): + return course_descriptor + # NOTE: The descriptors end up loading somewhat bottom up, which # breaks metadata inheritance via get_children(). Instead # (actually, in addition to, for now), we do a final inheritance pass From 95478922d06d87baf25979b35ef3c5b930e60124 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 11:34:06 -0500 Subject: [PATCH 2/3] Whitespace fixes --- common/lib/xmodule/xmodule/modulestore/xml.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 3e4cd4e899..17d6f04932 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -474,11 +474,11 @@ class XMLModuleStore(ModuleStoreBase): if category == "static_tab": for tab in course_descriptor.tabs or []: if tab.get('url_slug') == slug: - module.metadata['display_name'] = tab['name'] + module.metadata['display_name'] = tab['name'] module.metadata['data_dir'] = course_dir - self.modules[course_descriptor.id][module.location] = module + self.modules[course_descriptor.id][module.location] = module except Exception, e: - logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) + logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) system.error_tracker("ERROR: " + str(e)) def get_instance(self, course_id, location, depth=0): From 330a2eac80d2f8e235acb8420d3c2a60b132f8ae Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 11:38:34 -0500 Subject: [PATCH 3/3] Don't break the course if the grading policy is broken --- common/lib/xmodule/xmodule/course_module.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 8b4db799cb..0c854a8036 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -228,7 +228,11 @@ class CourseDescriptor(SequenceDescriptor): if policy_dir: paths = [policy_dir + '/grading_policy.json'] + paths - policy = json.loads(cls.read_grading_policy(paths, system)) + try: + policy = json.loads(cls.read_grading_policy(paths, system)) + except ValueError: + system.error_tracker("Unable to decode grading policy as json") + policy = None # cdodge: import the grading policy information that is on disk and put into the # descriptor 'definition' bucket as a dictionary so that it is persisted in the DB