From da3c3e5f20589ea8f8d16471f2cc358c5d6c3d78 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 9 Nov 2012 13:54:12 -0500 Subject: [PATCH] initial xlint implementation. Accumulate all import errors during XmlModuleStore importing. Also do checks post XmlModuleStore import and assert that the structure (course->chapter->sequential->vertical) is present in the courses. --- .../contentstore/management/commands/xlint.py | 6 ++-- common/lib/xmodule/xmodule/modulestore/xml.py | 7 ++-- .../xmodule/modulestore/xml_importer.py | 32 ++++++++++++++++++- common/lib/xmodule/xmodule/seq_module.py | 4 ++- rakefile | 4 ++- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/xlint.py b/cms/djangoapps/contentstore/management/commands/xlint.py index 355b639f2d..e8f7b248e4 100644 --- a/cms/djangoapps/contentstore/management/commands/xlint.py +++ b/cms/djangoapps/contentstore/management/commands/xlint.py @@ -9,8 +9,10 @@ unnamed_modules = 0 class Command(BaseCommand): help = \ -'''Verify the structure of courseware as to it's suitability for import''' - + ''' + Verify the structure of courseware as to it's suitability for import + To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)] + ''' def handle(self, *args, **options): if len(args) == 0: raise CommandError("import requires at least one argument: [...]") diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 6794703998..64ccf73d5e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -303,7 +303,7 @@ class XMLModuleStore(ModuleStoreBase): try: course_descriptor = self.load_course(course_dir, errorlog.tracker) except Exception as e: - msg = "Failed to load course '{0}': {1}".format(course_dir, str(e)) + msg = "ERROR: Failed to load course '{0}': {1}".format(course_dir, str(e)) log.exception(msg) errorlog.tracker(msg) @@ -337,7 +337,7 @@ class XMLModuleStore(ModuleStoreBase): with open(policy_path) as f: return json.load(f) except (IOError, ValueError) as err: - msg = "Error loading course policy from {0}".format(policy_path) + msg = "ERROR: loading course policy from {0}".format(policy_path) tracker(msg) log.warning(msg + " " + str(err)) return {} @@ -458,7 +458,8 @@ class XMLModuleStore(ModuleStoreBase): module.metadata['data_dir'] = course_dir 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): """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index a1739851ac..23eea58a97 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -227,6 +227,28 @@ def validate_category_hierarcy(module_store, course_id, parent_category, expecte def validate_module_structure(module_store): err_cnt = 0 warn_cnt = 0 + + print module_store.errored_courses + + # first count all errors and warnings as part of the XMLModuleStore import + for err_log in module_store._location_errors.itervalues(): + for err_log_entry in err_log.errors: + msg = err_log_entry[0] + if msg.startswith('ERROR:'): + err_cnt+=1 + else: + warn_cnt+=1 + + # then count outright all courses that failed to load at all + for err_log in module_store.errored_courses.itervalues(): + for err_log_entry in err_log.errors: + msg = err_log_entry[0] + print msg + if msg.startswith('ERROR:'): + err_cnt+=1 + else: + warn_cnt+=1 + for course_id in module_store.modules.keys(): # constrain that courses only have 'chapter' children err_cnt += validate_category_hierarcy(module_store, course_id, "course", "chapter") @@ -235,6 +257,14 @@ def validate_module_structure(module_store): # constrain that sequentials only have 'verticals' err_cnt += validate_category_hierarcy(module_store, course_id, "sequential", "vertical") - print "SUMMARY: {0} Errors {1} Warnings".format(err_cnt, warn_cnt) + print "\n\n------------------------------------------\nVALIDATION SUMMARY: {0} Errors {1} Warnings\n".format(err_cnt, warn_cnt) + + if err_cnt > 0: + print "This course is not suitable for importing. Please fix courseware according to specifications before importing." + elif warn_cnt > 0: + print "This course can be imported, but some errors may occur during the run of the course. It is recommend that you fix your courseware before importing" + else: + print "This course can be imported successfully." + diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 0ade3e0e7d..155ad99480 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -127,8 +127,10 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): for child in xml_object: try: children.append(system.process_xml(etree.tostring(child)).location.url()) - except: + except Exception, e: log.exception("Unable to load child when parsing Sequence. Continuing...") + if system.error_tracker is not None: + system.error_tracker("ERROR: " + str(e)) continue return {'children': children} diff --git a/rakefile b/rakefile index beb787c8c3..d8386fbda2 100644 --- a/rakefile +++ b/rakefile @@ -367,7 +367,9 @@ end namespace :cms do desc "Import course data within the given DATA_DIR variable" task :xlint do - if ENV['DATA_DIR'] + if ENV['DATA_DIR'] and ENV['COURSE_DIR'] + sh(django_admin(:cms, :dev, :xlint, ENV['DATA_DIR'], ENV['COURSE_DIR'])) + elsif ENV['DATA_DIR'] sh(django_admin(:cms, :dev, :xlint, ENV['DATA_DIR'])) else raise "Please specify a DATA_DIR variable that point to your data directory.\n" +