From 8a6b9a56662b078b18a25192a23139a07435e9cb Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 13:42:11 -0400 Subject: [PATCH 01/55] update lms readme --- lms/envs/README.txt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lms/envs/README.txt b/lms/envs/README.txt index 7a527b290f..6d1512e6f6 100644 --- a/lms/envs/README.txt +++ b/lms/envs/README.txt @@ -1,14 +1,16 @@ -Transitional for moving to new settings scheme. +Transitional for moving to new settings scheme. -To use: - django-admin.py runserver --settings=envs.dev --pythonpath=. +To use: + rake lms + or + django-admin.py runserver --settings=lms.envs.dev --pythonpath=. NOTE: Using manage.py will automatically run mitx/settings.py first, regardless of what you send it for an explicit --settings flag. It still works, but might -have odd side effects. Using django-admin.py avoids that problem. +have odd side effects. Using django-admin.py avoids that problem. django-admin.py is installed by default when you install Django. To use with gunicorn_django in debug mode: - gunicorn_django envs/dev.py + gunicorn_django lms/envs/dev.py From d43831e11c157188ea027945eea9ccb12a369297 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 13:42:54 -0400 Subject: [PATCH 02/55] add to overview of course.xml processing --- doc/overview.md | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/doc/overview.md b/doc/overview.md index 88ea3bdb5e..36e22e16eb 100644 --- a/doc/overview.md +++ b/doc/overview.md @@ -13,17 +13,17 @@ You should be familiar with the following. If you're not, go read some docs... - css - git - mako templates -- we use these instead of django templates, because they support embedding real python. - + ## Other relevant terms - CAPA -- lon-capa.org -- content management system that has defined a standard for online learning and assessment materials. Many of our materials follow this standard. - - TODO: add more details / link to relevant docs. lon-capa.org is not immediately intuitive. + - TODO: add more details / link to relevant docs. lon-capa.org is not immediately intuitive. - lcp = loncapa problem ## Parts of the system - - LMS -- Learning Management System. The student-facing parts of the system. Handles student accounts, displaying videos, tutorials, exercies, problems, etc. + - LMS -- Learning Management System. The student-facing parts of the system. Handles student accounts, displaying videos, tutorials, exercies, problems, etc. - CMS -- Course Management System. The instructor-facing parts of the system. Allows instructors to see and modify their course, add lectures, problems, reorder things, etc. @@ -42,7 +42,7 @@ You should be familiar with the following. If you're not, go read some docs... ## High Level Entities in the code -### Common libraries +### Common libraries - xmodule: generic learning modules. *x* can be sequence, video, template, html, vertical, capa, etc. These are the things that one puts inside sections @@ -51,7 +51,7 @@ You should be familiar with the following. If you're not, go read some docs... - XModuleDescriptor: This defines the problem and all data and UI needed to edit that problem. It is unaware of any student data, but can be used to retrieve an XModule, which is aware of that student state. - + - XModule: The XModule is a problem instance that is particular to a student. It knows how to render itself to html to display the problem, how to score itself, and how to handle ajax calls from the front end. @@ -59,19 +59,25 @@ You should be familiar with the following. If you're not, go read some docs... - Both XModule and XModuleDescriptor take system context parameters. These are named ModuleSystem and DescriptorSystem respectively. These help isolate the XModules from any interactions with external resources that they require. - + For instance, the DescriptorSystem has a function to load an XModuleDescriptor from a Location object, and the ModuleSystem knows how to render things, track events, and complain about 404s - - TODO: document the system context interface--it's different in `x_module.XModule.__init__` and in `x_module tests.py` (do this in the code, not here) + + - `course.xml` format. We use python setuptools to connect supported tags with the descriptors that handle them. See `common/lib/xmodule/setup.py`. There are checking and validation tools in `common/validate`. + + - the xml import+export functionality is in `xml_module.py:XmlDescriptor`, which is a mixin class that's used by the actual descriptor classes. + + - There is a distinction between descriptor _definitions_ that stay the same for any use of that descriptor (e.g. here is what a particular problem is), and _metadata_ describing how that descriptor is used (e.g. whether to allow checking of answers, due date, etc). When reading in `from_xml`, the code pulls out the metadata attributes into a separate structure, and puts it back on export. + - in `common/lib/xmodule` -- capa modules -- defines `LoncapaProblem` and many related things. +- capa modules -- defines `LoncapaProblem` and many related things. - in `common/lib/capa` -### LMS +### LMS -The LMS is a django site, with root in `lms/`. It runs in many different environments--the settings files are in `lms/envs`. +The LMS is a django site, with root in `lms/`. It runs in many different environments--the settings files are in `lms/envs`. - We use the Django Auth system, including the is_staff and is_superuser flags. User profiles and related code lives in `lms/djangoapps/student/`. There is support for groups of students (e.g. 'want emails about future courses', 'have unenrolled', etc) in `lms/djangoapps/student/models.py`. @@ -79,19 +85,19 @@ The LMS is a django site, with root in `lms/`. It runs in many different enviro - `lms/djangoapps/courseware/models.py` - Core rendering path: - - `lms/urls.py` points to `courseware.views.index`, which gets module info from the course xml file, pulls list of `StudentModule` objects for this user (to avoid multiple db hits). + - `lms/urls.py` points to `courseware.views.index`, which gets module info from the course xml file, pulls list of `StudentModule` objects for this user (to avoid multiple db hits). - Calls `render_accordion` to render the "accordion"--the display of the course structure. - To render the current module, calls `module_render.py:render_x_module()`, which gets the `StudentModule` instance, and passes the `StudentModule` state and other system context to the module constructor the get an instance of the appropriate module class for this user. - calls the module's `.get_html()` method. If the module has nested submodules, render_x_module() will be called again for each. - + - ajax calls go to `module_render.py:modx_dispatch()`, which passes it to the module's `handle_ajax()` function, and then updates the grade and state if they changed. - [This diagram](https://github.com/MITx/mitx/wiki/MITx-Architecture) visually shows how the clients communicate with problems + modules. - -- See `lms/urls.py` for the wirings of urls to views. + +- See `lms/urls.py` for the wirings of urls to views. - Tracking: there is support for basic tracking of client-side events in `lms/djangoapps/track`. @@ -110,7 +116,7 @@ environments, defined in `cms/envs`. - _mako_ -- we use this for templates, and have wrapper called mitxmako that makes mako look like the django templating calls. -We use a fork of django-pipeline to make sure that the js and css always reflect the latest `*.coffee` and `*.sass` files (We're hoping to get our changes merged in the official version soon). This works differently in development and production. Test uses the production settings. +We use a fork of django-pipeline to make sure that the js and css always reflect the latest `*.coffee` and `*.sass` files (We're hoping to get our changes merged in the official version soon). This works differently in development and production. Test uses the production settings. In production, the django `collectstatic` command recompiles everything and puts all the generated static files in a static/ dir. A starting point in the code is `django-pipeline/pipeline/packager.py:pack`. @@ -127,8 +133,6 @@ See `testing.md`. ## TODO: -- update lms/envs/README.txt - - describe our production environment - describe the front-end architecture, tools, etc. Starting point: `lms/static` From 63f34f2e70f4b930a02a36ed99cf54750fad05a4 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 13:58:19 -0400 Subject: [PATCH 03/55] Line length and doc string cleanups * no functionality changed in this commit. --- .../lib/xmodule/xmodule/backcompat_module.py | 20 +- common/lib/xmodule/xmodule/capa_module.py | 35 ++- common/lib/xmodule/xmodule/course_module.py | 11 +- common/lib/xmodule/xmodule/mako_module.py | 7 +- .../xmodule/xmodule/modulestore/__init__.py | 49 ++-- common/lib/xmodule/xmodule/raw_module.py | 2 +- common/lib/xmodule/xmodule/seq_module.py | 15 +- common/lib/xmodule/xmodule/x_module.py | 233 +++++++++++------- common/lib/xmodule/xmodule/xml_module.py | 34 ++- 9 files changed, 261 insertions(+), 145 deletions(-) diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index d379ced507..c040eca398 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -12,8 +12,8 @@ log = logging.getLogger(__name__) def process_includes(fn): """ Wraps a XModuleDescriptor.from_xml method, and modifies xml_data to replace - any immediate child items with the contents of the file that they are - supposed to include + any immediate child items with the contents of the file that they + are supposed to include """ @wraps(fn) def from_xml(cls, xml_data, system, org=None, course=None): @@ -25,15 +25,19 @@ def process_includes(fn): try: ifp = system.resources_fs.open(file) except Exception: - log.exception('Error in problem xml include: %s' % (etree.tostring(next_include, pretty_print=True))) - log.exception('Cannot find file %s in %s' % (file, dir)) + msg = 'Error in problem xml include: %s\n' % ( + etree.tostring(next_include, pretty_print=True)) + msg += 'Cannot find file %s in %s' % (file, dir) + log.exception(msg) raise try: # read in and convert to XML incxml = etree.XML(ifp.read()) except Exception: - log.exception('Error in problem xml include: %s' % (etree.tostring(next_include, pretty_print=True))) - log.exception('Cannot parse XML in %s' % (file)) + msg = 'Error in problem xml include: %s\n' % ( + etree.tostring(next_include, pretty_print=True)) + msg += 'Cannot parse XML in %s' % (file) + log.exception(msg) raise # insert new XML into tree in place of inlcude parent = next_include.getparent() @@ -50,8 +54,8 @@ class SemanticSectionDescriptor(XModuleDescriptor): @process_includes def from_xml(cls, xml_data, system, org=None, course=None): """ - Removes sections single child elements in favor of just embedding the child element - + Removes sections with single child elements in favor of just embedding + the child element """ xml_object = etree.fromstring(xml_data) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 263e062887..5ff0c13198 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -67,7 +67,8 @@ class ComplexEncoder(json.JSONEncoder): class CapaModule(XModule): ''' - An XModule implementing LonCapa format problems, implemented by way of capa.capa_problem.LoncapaProblem + An XModule implementing LonCapa format problems, implemented by way of + capa.capa_problem.LoncapaProblem ''' icon_class = 'problem' @@ -77,8 +78,10 @@ class CapaModule(XModule): js_module_name = "Problem" css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]} - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, instance_state=None, + shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, instance_state, + shared_state, **kwargs) self.attempts = 0 self.max_attempts = None @@ -133,7 +136,8 @@ class CapaModule(XModule): seed = None try: - self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), instance_state, seed=seed, system=self.system) + self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), + instance_state, seed=seed, system=self.system) except Exception: msg = 'cannot create LoncapaProblem %s' % self.location.url() log.exception(msg) @@ -141,15 +145,20 @@ class CapaModule(XModule): msg = '

%s

' % msg.replace('<', '<') msg += '

%s

' % traceback.format_exc().replace('<', '<') # create a dummy problem with error message instead of failing - problem_text = 'Problem %s has an error:%s' % (self.location.url(), msg) - self.lcp = LoncapaProblem(problem_text, self.location.html_id(), instance_state, seed=seed, system=self.system) + problem_text = ('' + 'Problem %s has an error:%s' % + (self.location.url(), msg)) + self.lcp = LoncapaProblem( + problem_text, self.location.html_id(), + instance_state, seed=seed, system=self.system) else: raise @property def rerandomize(self): """ - Property accessor that returns self.metadata['rerandomize'] in a canonical form + Property accessor that returns self.metadata['rerandomize'] in a + canonical form """ rerandomize = self.metadata.get('rerandomize', 'always') if rerandomize in ("", "always", "true"): @@ -203,7 +212,10 @@ class CapaModule(XModule): except Exception, err: if self.system.DEBUG: log.exception(err) - msg = '[courseware.capa.capa_module] Failed to generate HTML for problem %s' % (self.location.url()) + msg = ( + '[courseware.capa.capa_module] ' + 'Failed to generate HTML for problem %s' % + (self.location.url())) msg += '

Error:

%s

' % str(err).replace('<', '<') msg += '

%s

' % traceback.format_exc().replace('<', '<') html = msg @@ -215,8 +227,8 @@ class CapaModule(XModule): 'weight': self.weight, } - # We using strings as truthy values, because the terminology of the check button - # is context-specific. + # We using strings as truthy values, because the terminology of the + # check button is context-specific. check_button = "Grade" if self.max_attempts else "Check" reset_button = True save_button = True @@ -242,7 +254,8 @@ class CapaModule(XModule): if not self.lcp.done: reset_button = False - # We don't need a "save" button if infinite number of attempts and non-randomized + # We don't need a "save" button if infinite number of attempts and + # non-randomized if self.max_attempts is None and self.rerandomize != "always": save_button = False diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index a04324237c..05f440c0f8 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -17,11 +17,12 @@ class CourseDescriptor(SequenceDescriptor): try: self.start = time.strptime(self.metadata["start"], "%Y-%m-%dT%H:%M") except KeyError: - self.start = time.gmtime(0) # The epoch - log.critical("Course loaded without a start date. " + str(self.id)) - except ValueError, e: - self.start = time.gmtime(0) # The epoch - log.critical("Course loaded with a bad start date. " + str(self.id) + " '" + str(e) + "'") + self.start = time.gmtime(0) #The epoch + log.critical("Course loaded without a start date. %s", self.id) + except ValueError as e: + self.start = time.gmtime(0) #The epoch + log.critical("Course loaded with a bad start date. %s '%s'", + self.id, e) def has_started(self): return time.gmtime() > self.start diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index 9a90afb896..da620e4889 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -19,7 +19,9 @@ class MakoModuleDescriptor(XModuleDescriptor): def __init__(self, system, definition=None, **kwargs): if getattr(system, 'render_template', None) is None: - raise TypeError('{system} must have a render_template function in order to use a MakoDescriptor'.format(system=system)) + raise TypeError('{system} must have a render_template function' + ' in order to use a MakoDescriptor'.format( + system=system)) super(MakoModuleDescriptor, self).__init__(system, definition, **kwargs) def get_context(self): @@ -29,4 +31,5 @@ class MakoModuleDescriptor(XModuleDescriptor): return {'module': self} def get_html(self): - return self.system.render_template(self.mako_template, self.get_context()) + return self.system.render_template( + self.mako_template, self.get_context()) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 5527d4108e..ac03d92854 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -45,13 +45,17 @@ class Location(_LocationBase): """ return re.sub('_+', '_', INVALID_CHARS.sub('_', value)) - def __new__(_cls, loc_or_tag=None, org=None, course=None, category=None, name=None, revision=None): + def __new__(_cls, loc_or_tag=None, org=None, course=None, category=None, + name=None, revision=None): """ Create a new location that is a clone of the specifed one. location - Can be any of the following types: - string: should be of the form {tag}://{org}/{course}/{category}/{name}[/{revision}] + string: should be of the form + {tag}://{org}/{course}/{category}/{name}[/{revision}] + list: should be of the form [tag, org, course, category, name, revision] + dict: should be of the form { 'tag': tag, 'org': org, @@ -62,16 +66,19 @@ class Location(_LocationBase): } Location: another Location object - In both the dict and list forms, the revision is optional, and can be ommitted. + In both the dict and list forms, the revision is optional, and can be + ommitted. - Components must be composed of alphanumeric characters, or the characters '_', '-', and '.' + Components must be composed of alphanumeric characters, or the + characters '_', '-', and '.' - Components may be set to None, which may be interpreted by some contexts to mean - wildcard selection + Components may be set to None, which may be interpreted by some contexts + to mean wildcard selection """ - if org is None and course is None and category is None and name is None and revision is None: + if (org is None and course is None and category is None and + name is None and revision is None): location = loc_or_tag else: location = (loc_or_tag, org, course, category, name, revision) @@ -131,9 +138,11 @@ class Location(_LocationBase): def html_id(self): """ - Return a string with a version of the location that is safe for use in html id attributes + Return a string with a version of the location that is safe for use in + html id attributes """ - return "-".join(str(v) for v in self.list() if v is not None).replace('.', '_') + return "-".join(str(v) for v in self.list() + if v is not None).replace('.', '_') def dict(self): """ @@ -154,7 +163,8 @@ class Location(_LocationBase): class ModuleStore(object): """ - An abstract interface for a database backend that stores XModuleDescriptor instances + An abstract interface for a database backend that stores XModuleDescriptor + instances """ def get_item(self, location, depth=0): """ @@ -164,13 +174,16 @@ class ModuleStore(object): If any segment of the location is None except revision, raises xmodule.modulestore.exceptions.InsufficientSpecificationError - If no object is found at that location, raises xmodule.modulestore.exceptions.ItemNotFoundError + + If no object is found at that location, raises + xmodule.modulestore.exceptions.ItemNotFoundError location: Something that can be passed to Location - depth (int): An argument that some module stores may use to prefetch descendents of the queried modules - for more efficient results later in the request. The depth is counted in the number of - calls to get_children() to cache. None indicates to cache all descendents + depth (int): An argument that some module stores may use to prefetch + descendents of the queried modules for more efficient results later + in the request. The depth is counted in the number of calls to + get_children() to cache. None indicates to cache all descendents """ raise NotImplementedError @@ -182,9 +195,10 @@ class ModuleStore(object): location: Something that can be passed to Location - depth: An argument that some module stores may use to prefetch descendents of the queried modules - for more efficient results later in the request. The depth is counted in the number of calls - to get_children() to cache. None indicates to cache all descendents + depth: An argument that some module stores may use to prefetch + descendents of the queried modules for more efficient results later + in the request. The depth is counted in the number of calls to + get_children() to cache. None indicates to cache all descendents """ raise NotImplementedError @@ -228,4 +242,3 @@ class ModuleStore(object): in this modulestore. ''' raise NotImplementedError - diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 2794e27dd6..2a4c04e512 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -8,7 +8,7 @@ log = logging.getLogger(__name__) class RawDescriptor(MakoModuleDescriptor, XmlDescriptor): """ - Module that provides a raw editing view of it's data and children + Module that provides a raw editing view of its data and children """ mako_template = "widgets/raw-edit.html" diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index b39292c2ca..d435be627b 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -20,12 +20,15 @@ class_priority = ['video', 'problem'] class SequenceModule(XModule): ''' Layout module which lays out content in a temporal sequence ''' - js = {'coffee': [resource_string(__name__, 'js/src/sequence/display.coffee')]} + js = {'coffee': [resource_string(__name__, + 'js/src/sequence/display.coffee')]} css = {'scss': [resource_string(__name__, 'css/sequence/display.scss')]} js_module_name = "Sequence" - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, instance_state=None, + shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, + instance_state, shared_state, **kwargs) self.position = 1 if instance_state is not None: @@ -92,7 +95,8 @@ class SequenceModule(XModule): self.rendered = True def get_icon_class(self): - child_classes = set(child.get_icon_class() for child in self.get_children()) + child_classes = set(child.get_icon_class() + for child in self.get_children()) new_class = 'other' for c in class_priority: if c in child_classes: @@ -114,5 +118,6 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): def definition_to_xml(self, resource_fs): xml_object = etree.Element('sequential') for child in self.get_children(): - xml_object.append(etree.fromstring(child.export_to_xml(resource_fs))) + xml_object.append( + etree.fromstring(child.export_to_xml(resource_fs))) return xml_object diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 996d31a83d..0c6d99fcf4 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -31,23 +31,28 @@ class Plugin(object): def load_class(cls, identifier, default=None): """ Loads a single class instance specified by identifier. If identifier - specifies more than a single class, then logs a warning and returns the first - class identified. + specifies more than a single class, then logs a warning and returns the + first class identified. - If default is not None, will return default if no entry_point matching identifier - is found. Otherwise, will raise a ModuleMissingError + If default is not None, will return default if no entry_point matching + identifier is found. Otherwise, will raise a ModuleMissingError """ if cls._plugin_cache is None: cls._plugin_cache = {} if identifier not in cls._plugin_cache: identifier = identifier.lower() - classes = list(pkg_resources.iter_entry_points(cls.entry_point, name=identifier)) + classes = list(pkg_resources.iter_entry_points( + cls.entry_point, name=identifier)) + if len(classes) > 1: - log.warning("Found multiple classes for {entry_point} with identifier {id}: {classes}. Returning the first one.".format( + log.warning("Found multiple classes for {entry_point} with " + "identifier {id}: {classes}. " + "Returning the first one.".format( entry_point=cls.entry_point, id=identifier, - classes=", ".join(class_.module_name for class_ in classes))) + classes=", ".join( + class_.module_name for class_ in classes))) if len(classes) == 0: if default is not None: @@ -79,9 +84,12 @@ class HTMLSnippet(object): def get_javascript(cls): """ Return a dictionary containing some of the following keys: + coffee: A list of coffeescript fragments that should be compiled and placed on the page - js: A list of javascript fragments that should be included on the page + + js: A list of javascript fragments that should be included on the + page All of these will be loaded onto the page in the CMS """ @@ -91,12 +99,15 @@ class HTMLSnippet(object): def get_css(cls): """ Return a dictionary containing some of the following keys: - css: A list of css fragments that should be applied to the html contents - of the snippet - sass: A list of sass fragments that should be applied to the html contents - of the snippet - scss: A list of scss fragments that should be applied to the html contents - of the snippet + + css: A list of css fragments that should be applied to the html + contents of the snippet + + sass: A list of sass fragments that should be applied to the html + contents of the snippet + + scss: A list of scss fragments that should be applied to the html + contents of the snippet """ return cls.css @@ -104,47 +115,70 @@ class HTMLSnippet(object): """ Return the html used to display this snippet """ - raise NotImplementedError("get_html() must be provided by specific modules - not present in {0}" + raise NotImplementedError( + "get_html() must be provided by specific modules - not present in {0}" .format(self.__class__)) class XModule(HTMLSnippet): ''' Implements a generic learning module. - Subclasses must at a minimum provide a definition for get_html in order to be displayed to users. + Subclasses must at a minimum provide a definition for get_html in order + to be displayed to users. See the HTML module for a simple example. ''' - # The default implementation of get_icon_class returns the icon_class attribute of the class - # This attribute can be overridden by subclasses, and the function can also be overridden - # if the icon class depends on the data in the module + # The default implementation of get_icon_class returns the icon_class + # attribute of the class + # + # This attribute can be overridden by subclasses, and + # the function can also be overridden if the icon class depends on the data + # in the module icon_class = 'other' - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + def __init__(self, system, location, definition, + instance_state=None, shared_state=None, **kwargs): ''' Construct a new xmodule system: A ModuleSystem allowing access to external resources + location: Something Location-like that identifies this xmodule - definition: A dictionary containing 'data' and 'children'. Both are optional - 'data': is JSON-like (string, dictionary, list, bool, or None, optionally nested). - This defines all of the data necessary for a problem to display that is intrinsic to the problem. - It should not include any data that would vary between two courses using the same problem + + definition: A dictionary containing 'data' and 'children'. Both are + optional + + 'data': is JSON-like (string, dictionary, list, bool, or None, + optionally nested). + + This defines all of the data necessary for a problem to display + that is intrinsic to the problem. It should not include any + data that would vary between two courses using the same problem (due dates, grading policy, randomization, etc.) - 'children': is a list of Location-like values for child modules that this module depends on - instance_state: A string of serialized json that contains the state of this module for - current student accessing the system, or None if no state has been saved - shared_state: A string of serialized json that contains the state that is shared between - this module and any modules of the same type with the same shared_state_key. This - state is only shared per-student, not across different students - kwargs: Optional arguments. Subclasses should always accept kwargs and pass them - to the parent class constructor. + + 'children': is a list of Location-like values for child modules that + this module depends on + + instance_state: A string of serialized json that contains the state of + this module for current student accessing the system, or None if + no state has been saved + + shared_state: A string of serialized json that contains the state that + is shared between this module and any modules of the same type with + the same shared_state_key. This state is only shared per-student, + not across different students + + kwargs: Optional arguments. Subclasses should always accept kwargs and + pass them to the parent class constructor. + Current known uses of kwargs: - metadata: SCAFFOLDING - This dictionary will be split into several different types of metadata - in the future (course policy, modification history, etc). - A dictionary containing data that specifies information that is particular - to a problem in the context of a course + + metadata: SCAFFOLDING - This dictionary will be split into + several different types of metadata in the future (course + policy, modification history, etc). A dictionary containing + data that specifies information that is particular to a + problem in the context of a course ''' self.system = system self.location = Location(location) @@ -217,16 +251,21 @@ class XModule(HTMLSnippet): def max_score(self): ''' Maximum score. Two notes: - * This is generic; in abstract, a problem could be 3/5 points on one randomization, and 5/7 on another - * In practice, this is a Very Bad Idea, and (a) will break some code in place (although that code - should get fixed), and (b) break some analytics we plan to put in place. + + * This is generic; in abstract, a problem could be 3/5 points on one + randomization, and 5/7 on another + + * In practice, this is a Very Bad Idea, and (a) will break some code + in place (although that code should get fixed), and (b) break some + analytics we plan to put in place. ''' return None def get_progress(self): - ''' Return a progress.Progress object that represents how far the student has gone - in this module. Must be implemented to get correct progress tracking behavior in - nesting modules like sequence and vertical. + ''' Return a progress.Progress object that represents how far the + student has gone in this module. Must be implemented to get correct + progress tracking behavior in nesting modules like sequence and + vertical. If this module has no notion of progress, return None. ''' @@ -240,13 +279,14 @@ class XModule(HTMLSnippet): class XModuleDescriptor(Plugin, HTMLSnippet): """ - An XModuleDescriptor is a specification for an element of a course. This could - be a problem, an organizational element (a group of content), or a segment of video, - for example. + An XModuleDescriptor is a specification for an element of a course. This + could be a problem, an organizational element (a group of content), or a + segment of video, for example. - XModuleDescriptors are independent and agnostic to the current student state on a - problem. They handle the editing interface used by instructors to create a problem, - and can generate XModules (which do know about student state). + XModuleDescriptors are independent and agnostic to the current student state + on a problem. They handle the editing interface used by instructors to + create a problem, and can generate XModules (which do know about student + state). """ entry_point = "xmodule.v1" module_class = XModule @@ -255,46 +295,58 @@ class XModuleDescriptor(Plugin, HTMLSnippet): inheritable_metadata = ( 'graded', 'start', 'due', 'graceperiod', 'showanswer', 'rerandomize', - # This is used by the XMLModuleStore to provide for locations for static files, - # and will need to be removed when that code is removed + # TODO: This is used by the XMLModuleStore to provide for locations for + # static files, and will need to be removed when that code is removed 'data_dir' ) - # A list of descriptor attributes that must be equal for the descriptors to be - # equal - equality_attributes = ('definition', 'metadata', 'location', 'shared_state_key', '_inherited_metadata') + # A list of descriptor attributes that must be equal for the descriptors to + # be equal + equality_attributes = ('definition', 'metadata', 'location', + 'shared_state_key', '_inherited_metadata') - # ============================= STRUCTURAL MANIPULATION =========================== + # ============================= STRUCTURAL MANIPULATION =================== def __init__(self, system, definition=None, **kwargs): """ Construct a new XModuleDescriptor. The only required arguments are the - system, used for interaction with external resources, and the definition, - which specifies all the data needed to edit and display the problem (but none - of the associated metadata that handles recordkeeping around the problem). + system, used for interaction with external resources, and the + definition, which specifies all the data needed to edit and display the + problem (but none of the associated metadata that handles recordkeeping + around the problem). - This allows for maximal flexibility to add to the interface while preserving - backwards compatibility. + This allows for maximal flexibility to add to the interface while + preserving backwards compatibility. - system: An XModuleSystem for interacting with external resources - definition: A dict containing `data` and `children` representing the problem definition + system: A DescriptorSystem for interacting with external resources + + definition: A dict containing `data` and `children` representing the + problem definition Current arguments passed in kwargs: - location: A xmodule.modulestore.Location object indicating the name and ownership of this problem - shared_state_key: The key to use for sharing StudentModules with other - modules of this type + + location: A xmodule.modulestore.Location object indicating the name + and ownership of this problem + + shared_state_key: The key to use for sharing StudentModules with + other modules of this type + metadata: A dictionary containing the following optional keys: - goals: A list of strings of learning goals associated with this module - display_name: The name to use for displaying this module to the user + goals: A list of strings of learning goals associated with this + module + display_name: The name to use for displaying this module to the + user format: The format of this module ('Homework', 'Lab', etc) graded (bool): Whether this module is should be graded or not start (string): The date for which this module will be available due (string): The due date for this module - graceperiod (string): The amount of grace period to allow when enforcing the due date + graceperiod (string): The amount of grace period to allow when + enforcing the due date showanswer (string): When to show answers for this module - rerandomize (string): When to generate a newly randomized instance of the module data + rerandomize (string): When to generate a newly randomized + instance of the module data """ self.system = system self.metadata = kwargs.get('metadata', {}) @@ -321,7 +373,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet): self.metadata[attr] = metadata[attr] def get_children(self): - """Returns a list of XModuleDescriptor instances for the children of this module""" + """Returns a list of XModuleDescriptor instances for the children of + this module""" if self._child_instances is None: self._child_instances = [] for child_loc in self.definition.get('children', []): @@ -333,8 +386,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet): def xmodule_constructor(self, system): """ - Returns a constructor for an XModule. This constructor takes two arguments: - instance_state and shared_state, and returns a fully nstantiated XModule + Returns a constructor for an XModule. This constructor takes two + arguments: instance_state and shared_state, and returns a fully + instantiated XModule """ return partial( self.module_class, @@ -344,7 +398,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet): metadata=self.metadata ) - # ================================= JSON PARSING =================================== + # ================================= JSON PARSING =========================== @staticmethod def load_from_json(json_data, system, default_class=None): """ @@ -366,13 +420,14 @@ class XModuleDescriptor(Plugin, HTMLSnippet): Creates an instance of this descriptor from the supplied json_data. This may be overridden by subclasses - json_data: A json object specifying the definition and any optional keyword arguments for - the XModuleDescriptor - system: An XModuleSystem for interacting with external resources + json_data: A json object specifying the definition and any optional + keyword arguments for the XModuleDescriptor + + system: A DescriptorSystem for interacting with external resources """ return cls(system=system, **json_data) - # ================================= XML PARSING ==================================== + # ================================= XML PARSING ============================ @staticmethod def load_from_xml(xml_data, system, @@ -487,24 +542,33 @@ class ModuleSystem(object): ''' def __init__(self, ajax_url, track_function, get_module, render_template, replace_urls, - user=None, filestore=None, debug=False, xqueue_callback_url=None): + user=None, filestore=None, debug=False, + xqueue_callback_url=None): ''' Create a closure around the system environment. ajax_url - the url where ajax calls to the encapsulating module go. + track_function - function of (event_type, event), intended for logging or otherwise tracking the event. TODO: Not used, and has inconsistent args in different files. Update or remove. + get_module - function that takes (location) and returns a corresponding - module instance object. - render_template - a function that takes (template_file, context), and returns - rendered html. - user - The user to base the random number generator seed off of for this request - filestore - A filestore ojbect. Defaults to an instance of OSFS based at - settings.DATA_DIR. + module instance object. + + render_template - a function that takes (template_file, context), and + returns rendered html. + + user - The user to base the random number generator seed off of for this + request + + filestore - A filestore ojbect. Defaults to an instance of OSFS based + at settings.DATA_DIR. + replace_urls - TEMPORARY - A function like static_replace.replace_urls - that capa_module can use to fix up the static urls in ajax results. + that capa_module can use to fix up the static urls in + ajax results. ''' self.ajax_url = ajax_url self.xqueue_callback_url = xqueue_callback_url @@ -529,4 +593,3 @@ class ModuleSystem(object): def __str__(self): return str(self.__dict__) - diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index a7fc686e45..49e0f79976 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -13,13 +13,19 @@ log = logging.getLogger(__name__) # TODO (cpennington): This was implemented in an attempt to improve performance, # but the actual improvement wasn't measured (and it was implemented late at night). # We should check if it hurts, and whether there's a better way of doing lazy loading + class LazyLoadingDict(MutableMapping): """ - A dictionary object that lazily loads it's contents from a provided - function on reads (of members that haven't already been set) + A dictionary object that lazily loads its contents from a provided + function on reads (of members that haven't already been set). """ def __init__(self, loader): + ''' + On the first read from this dictionary, it will call loader() to + populate its contents. loader() must return something dict-like. Any + elements set before the first read will be preserved. + ''' self._contents = {} self._loaded = False self._loader = loader @@ -70,10 +76,12 @@ _AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata') class AttrMap(_AttrMapBase): """ - A class that specifies a metadata_key, a function to transform an xml attribute to be placed in that key, - and to transform that key value + A class that specifies a metadata_key, a function to transform an xml + attribute to be placed in that key, and to transform that key value """ - def __new__(_cls, metadata_key, to_metadata=lambda x: x, from_metadata=lambda x: x): + def __new__(_cls, metadata_key, + to_metadata=lambda x: x, + from_metadata=lambda x: x): return _AttrMapBase.__new__(_cls, metadata_key, to_metadata, from_metadata) @@ -93,7 +101,9 @@ class XmlDescriptor(XModuleDescriptor): # A dictionary mapping xml attribute names to functions of the value # that return the metadata key and value xml_attribute_map = { - 'graded': AttrMap('graded', lambda val: val == 'true', lambda val: str(val).lower()), + 'graded': AttrMap('graded', + lambda val: val == 'true', + lambda val: str(val).lower()), 'name': AttrMap('display_name'), } @@ -105,12 +115,14 @@ class XmlDescriptor(XModuleDescriptor): xml_object: An etree Element """ - raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__) + raise NotImplementedError( + "%s does not implement definition_from_xml" % cls.__name__) @classmethod def clean_metadata_from_xml(cls, xml_object): """ - Remove any attribute named in self.metadata_attributes from the supplied xml_object + Remove any attribute named in cls.metadata_attributes from the supplied + xml_object """ for attr in cls.metadata_attributes: if xml_object.get(attr) is not None: @@ -134,7 +146,7 @@ class XmlDescriptor(XModuleDescriptor): xml_data: A string of xml that will be translated into data and children for this module - system: An XModuleSystem for interacting with external resources + system: A DescriptorSystem for interacting with external resources org and course are optional strings that will be used in the generated modules url identifiers """ @@ -157,6 +169,7 @@ class XmlDescriptor(XModuleDescriptor): else: filepath = cls._format_filepath(xml_object.tag, filename) + # VS[compat] # TODO (cpennington): If the file doesn't exist at the right path, # give the class a chance to fix it up. The file will be written out again # in the correct format. @@ -243,4 +256,5 @@ class XmlDescriptor(XModuleDescriptor): """ Return a new etree Element object created from this modules definition. """ - raise NotImplementedError("%s does not implement definition_to_xml" % self.__class__.__name__) + raise NotImplementedError( + "%s does not implement definition_to_xml" % self.__class__.__name__) From 5f84e6192520b31170074616bde1dffe672fb910 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:06:18 -0400 Subject: [PATCH 04/55] Add hook for error handling during xml import * add error_handler member to DescriptorSystem * call it where import errors happen * also includes some refactoring in xml.py * some more line length and docstring cleanups --- .../lib/xmodule/xmodule/backcompat_module.py | 2 + common/lib/xmodule/xmodule/errorhandlers.py | 25 +++ common/lib/xmodule/xmodule/modulestore/xml.py | 200 +++++++++++------- common/lib/xmodule/xmodule/raw_module.py | 11 +- common/lib/xmodule/xmodule/x_module.py | 111 +++++++--- common/lib/xmodule/xmodule/xml_module.py | 28 ++- 6 files changed, 251 insertions(+), 126 deletions(-) create mode 100644 common/lib/xmodule/xmodule/errorhandlers.py diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index c040eca398..456a2c2b07 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -29,6 +29,7 @@ def process_includes(fn): etree.tostring(next_include, pretty_print=True)) msg += 'Cannot find file %s in %s' % (file, dir) log.exception(msg) + system.error_handler(msg) raise try: # read in and convert to XML @@ -38,6 +39,7 @@ def process_includes(fn): etree.tostring(next_include, pretty_print=True)) msg += 'Cannot parse XML in %s' % (file) log.exception(msg) + system.error_handler(msg) raise # insert new XML into tree in place of inlcude parent = next_include.getparent() diff --git a/common/lib/xmodule/xmodule/errorhandlers.py b/common/lib/xmodule/xmodule/errorhandlers.py new file mode 100644 index 0000000000..6840d9ff74 --- /dev/null +++ b/common/lib/xmodule/xmodule/errorhandlers.py @@ -0,0 +1,25 @@ +import sys + +def strict_error_handler(msg, exc_info=None): + ''' + Do not let errors pass. If exc_info is not None, ignore msg, and just + re-raise. Otherwise, check if we are in an exception-handling context. + If so, re-raise. Otherwise, raise Exception(msg). + + Meant for use in validation, where any errors should trap. + ''' + if exc_info is not None: + raise exc_info[0], exc_info[1], exc_info[2] + + # Check if there is an exception being handled somewhere up the stack + if sys.exc_info() != (None, None, None): + raise + + raise Exception(msg) + + +def ignore_errors_handler(msg, exc_info=None): + '''Ignore all errors, relying on the caller to workaround. + Meant for use in the LMS, where an error in one part of the course + shouldn't bring down the whole system''' + pass diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 3981009cef..8965f3d028 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -3,6 +3,7 @@ from fs.osfs import OSFS from importlib import import_module from lxml import etree from path import path +from xmodule.errorhandlers import strict_error_handler from xmodule.x_module import XModuleDescriptor, XMLParsingSystem from xmodule.mako_module import MakoDescriptorSystem from cStringIO import StringIO @@ -18,32 +19,112 @@ etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False, log = logging.getLogger('mitx.' + __name__) -# TODO (cpennington): Remove this once all fall 2012 courses have been imported into the cms from xml +# VS[compat] +# TODO (cpennington): Remove this once all fall 2012 courses have been imported +# into the cms from xml def clean_out_mako_templating(xml_string): xml_string = xml_string.replace('%include', 'include') xml_string = re.sub("(?m)^\s*%.*$", '', xml_string) return xml_string +class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): + def __init__(self, xmlstore, org, course, course_dir, + error_handler): + """ + A class that handles loading from xml. Does some munging to ensure that + all elements have unique slugs. + + xmlstore: the XMLModuleStore to store the loaded modules in + """ + self.unnamed_modules = 0 + self.used_slugs = set() + + def process_xml(xml): + try: + # VS[compat] + # TODO (cpennington): Remove this once all fall 2012 courses + # have been imported into the cms from xml + xml = clean_out_mako_templating(xml) + xml_data = etree.fromstring(xml) + except: + log.exception("Unable to parse xml: {xml}".format(xml=xml)) + raise + + # TODO (vshnayder): is the slug munging permanent, or also intended + # to be taken out? + if xml_data.get('slug') is None: + if xml_data.get('name'): + slug = Location.clean(xml_data.get('name')) + else: + self.unnamed_modules += 1 + slug = '{tag}_{count}'.format(tag=xml_data.tag, + count=self.unnamed_modules) + + if slug in self.used_slugs: + self.unnamed_modules += 1 + slug = '{slug}_{count}'.format(slug=slug, + count=self.unnamed_modules) + + self.used_slugs.add(slug) + # log.debug('-> slug=%s' % slug) + xml_data.set('slug', slug) + + module = XModuleDescriptor.load_from_xml( + etree.tostring(xml_data), self, org, + course, xmlstore.default_class) + + log.debug('==> importing module location %s' % repr(module.location)) + module.metadata['data_dir'] = course_dir + + xmlstore.modules[module.location] = module + + if xmlstore.eager: + module.get_children() + return module + + system_kwargs = dict( + render_template=lambda: '', + load_item=xmlstore.get_item, + resources_fs=OSFS(xmlstore.data_dir / course_dir), + process_xml=process_xml, + error_handler=error_handler, + ) + MakoDescriptorSystem.__init__(self, **system_kwargs) + XMLParsingSystem.__init__(self, **system_kwargs) + + class XMLModuleStore(ModuleStore): """ An XML backed ModuleStore """ - def __init__(self, data_dir, default_class=None, eager=False, course_dirs=None): + def __init__(self, data_dir, default_class=None, eager=False, + course_dirs=None, + error_handler=strict_error_handler): """ Initialize an XMLModuleStore from data_dir data_dir: path to data directory containing the course directories - default_class: dot-separated string defining the default descriptor class to use if non is specified in entry_points - eager: If true, load the modules children immediately to force the entire course tree to be parsed - course_dirs: If specified, the list of course_dirs to load. Otherwise, load - all course dirs + + default_class: dot-separated string defining the default descriptor + class to use if none is specified in entry_points + + eager: If true, load the modules children immediately to force the + entire course tree to be parsed + + course_dirs: If specified, the list of course_dirs to load. Otherwise, + load all course dirs + + error_handler: The error handler used here and in the underlying + DescriptorSystem. By default, raise exceptions for all errors. + See the comments in x_module.py:DescriptorSystem """ self.eager = eager self.data_dir = path(data_dir) self.modules = {} # location -> XModuleDescriptor self.courses = {} # course_dir -> XModuleDescriptor for the course + self.error_handler = error_handler if default_class is None: self.default_class = None @@ -54,110 +135,63 @@ class XMLModuleStore(ModuleStore): self.default_class = class_ # TODO (cpennington): We need a better way of selecting specific sets of debug messages to enable. These were drowning out important messages - #log.debug('XMLModuleStore: eager=%s, data_dir = %s' % (eager, self.data_dir)) - #log.debug('default_class = %s' % self.default_class) + log.debug('XMLModuleStore: eager=%s, data_dir = %s' % (eager, self.data_dir)) + log.debug('default_class = %s' % self.default_class) - for course_dir in os.listdir(self.data_dir): - if course_dirs is not None and course_dir not in course_dirs: - continue - - if not os.path.exists(self.data_dir / course_dir / "course.xml"): - continue + # If we are specifically asked for missing courses, that should + # be an error. If we are asked for "all" courses, find the ones + # that have a course.xml + if course_dirs is None: + course_dirs = [d for d in os.listdir(self.data_dir) if + os.path.exists(self.data_dir / d / "course.xml")] + for course_dir in course_dirs: try: course_descriptor = self.load_course(course_dir) self.courses[course_dir] = course_descriptor except: - log.exception("Failed to load course %s" % course_dir) + msg = "Failed to load course '%s'" % course_dir + log.exception(msg) + error_handler(msg) + def load_course(self, course_dir): """ Load a course into this module store course_path: Course directory name + + returns a CourseDescriptor for the course """ with open(self.data_dir / course_dir / "course.xml") as course_file: - # TODO (cpennington): Remove this once all fall 2012 courses have been imported - # into the cms from xml + # VS[compat] + # TODO (cpennington): Remove this once all fall 2012 courses have + # been imported into the cms from xml course_file = StringIO(clean_out_mako_templating(course_file.read())) course_data = etree.parse(course_file).getroot() org = course_data.get('org') if org is None: - log.error( - "No 'org' attribute set for course in {dir}. Using default 'edx'".format( - dir=course_dir)) + log.error("No 'org' attribute set for course in {dir}. " + "Using default 'edx'".format(dir=course_dir)) org = 'edx' course = course_data.get('course') if course is None: - log.error( - "No 'course' attribute set for course in {dir}. Using default '{default}'".format( - dir=course_dir, - default=course_dir - )) + log.error("No 'course' attribute set for course in {dir}." + " Using default '{default}'".format( + dir=course_dir, + default=course_dir + )) course = course_dir - class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): - def __init__(self, xmlstore): - """ - xmlstore: the XMLModuleStore to store the loaded modules in - """ - self.unnamed_modules = 0 - self.used_slugs = set() + system = ImportSystem(self, org, course, course_dir, + self.error_handler) - def process_xml(xml): - try: - # TODO (cpennington): Remove this once all fall 2012 courses - # have been imported into the cms from xml - xml = clean_out_mako_templating(xml) - xml_data = etree.fromstring(xml) - except: - log.exception("Unable to parse xml: {xml}".format(xml=xml)) - raise - if xml_data.get('slug') is None: - if xml_data.get('name'): - slug = Location.clean(xml_data.get('name')) - else: - self.unnamed_modules += 1 - slug = '{tag}_{count}'.format(tag=xml_data.tag, - count=self.unnamed_modules) - - if slug in self.used_slugs: - self.unnamed_modules += 1 - slug = '{slug}_{count}'.format(slug=slug, - count=self.unnamed_modules) - - self.used_slugs.add(slug) - # log.debug('-> slug=%s' % slug) - xml_data.set('slug', slug) - - module = XModuleDescriptor.load_from_xml( - etree.tostring(xml_data), self, org, - course, xmlstore.default_class) - log.debug('==> importing module location %s' % repr(module.location)) - module.metadata['data_dir'] = course_dir - - xmlstore.modules[module.location] = module - - if xmlstore.eager: - module.get_children() - return module - - system_kwargs = dict( - render_template=lambda: '', - load_item=xmlstore.get_item, - resources_fs=OSFS(xmlstore.data_dir / course_dir), - process_xml=process_xml - ) - MakoDescriptorSystem.__init__(self, **system_kwargs) - XMLParsingSystem.__init__(self, **system_kwargs) - - - course_descriptor = ImportSystem(self).process_xml(etree.tostring(course_data)) + course_descriptor = system.process_xml(etree.tostring(course_data)) log.debug('========> Done with course import') return course_descriptor @@ -169,7 +203,9 @@ class XMLModuleStore(ModuleStore): If any segment of the location is None except revision, raises xmodule.modulestore.exceptions.InsufficientSpecificationError - If no object is found at that location, raises xmodule.modulestore.exceptions.ItemNotFoundError + + If no object is found at that location, raises + xmodule.modulestore.exceptions.ItemNotFoundError location: Something that can be passed to Location """ diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 2a4c04e512..d8c18b251a 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -31,8 +31,11 @@ class RawDescriptor(MakoModuleDescriptor, XmlDescriptor): except etree.XMLSyntaxError as err: lines = self.definition['data'].split('\n') line, offset = err.position - log.exception("Unable to create xml for problem {loc}. Context: '{context}'".format( - context=lines[line-1][offset - 40:offset + 40], - loc=self.location - )) + msg = ("Unable to create xml for problem {loc}. " + "Context: '{context}'".format( + context=lines[line-1][offset - 40:offset + 40], + loc=self.location)) + log.exception(msg) + self.system.error_handler(msg) + # no workaround possible, so just re-raise raise diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 0c6d99fcf4..3e564053bd 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -203,13 +203,16 @@ class XModule(HTMLSnippet): Return module instances for all the children of this module. ''' if self._loaded_children is None: - self._loaded_children = [self.system.get_module(child) for child in self.definition.get('children', [])] + self._loaded_children = [ + self.system.get_module(child) + for child in self.definition.get('children', [])] + return self._loaded_children def get_display_items(self): ''' - Returns a list of descendent module instances that will display immediately - inside this module + Returns a list of descendent module instances that will display + immediately inside this module ''' items = [] for child in self.get_children(): @@ -219,8 +222,8 @@ class XModule(HTMLSnippet): def displayable_items(self): ''' - Returns list of displayable modules contained by this module. If this module - is visible, should return [self] + Returns list of displayable modules contained by this module. If this + module is visible, should return [self] ''' return [self] @@ -439,16 +442,19 @@ class XModuleDescriptor(Plugin, HTMLSnippet): on the contents of xml_data. xml_data must be a string containing valid xml + system is an XMLParsingSystem - org and course are optional strings that will be used in the generated modules - url identifiers + + org and course are optional strings that will be used in the generated + modules url identifiers """ class_ = XModuleDescriptor.load_class( etree.fromstring(xml_data).tag, default_class ) - # leave next line in code, commented out - useful for low-level debugging - # log.debug('[XModuleDescriptor.load_from_xml] tag=%s, class_=%s' % (etree.fromstring(xml_data).tag,class_)) + # leave next line, commented out - useful for low-level debugging + log.debug('[XModuleDescriptor.load_from_xml] tag=%s, class_=%s' % ( + etree.fromstring(xml_data).tag,class_)) return class_.from_xml(xml_data, system, org, course) @classmethod @@ -457,35 +463,42 @@ class XModuleDescriptor(Plugin, HTMLSnippet): Creates an instance of this descriptor from the supplied xml_data. This may be overridden by subclasses - xml_data: A string of xml that will be translated into data and children for - this module + xml_data: A string of xml that will be translated into data and children + for this module + system is an XMLParsingSystem - org and course are optional strings that will be used in the generated modules - url identifiers + + org and course are optional strings that will be used in the generated + module's url identifiers """ - raise NotImplementedError('Modules must implement from_xml to be parsable from xml') + raise NotImplementedError( + 'Modules must implement from_xml to be parsable from xml') def export_to_xml(self, resource_fs): """ - Returns an xml string representing this module, and all modules underneath it. - May also write required resources out to resource_fs + Returns an xml string representing this module, and all modules + underneath it. May also write required resources out to resource_fs - Assumes that modules have single parantage (that no module appears twice in the same course), - and that it is thus safe to nest modules as xml children as appropriate. + Assumes that modules have single parentage (that no module appears twice + in the same course), and that it is thus safe to nest modules as xml + children as appropriate. - The returned XML should be able to be parsed back into an identical XModuleDescriptor - using the from_xml method with the same system, org, and course + The returned XML should be able to be parsed back into an identical + XModuleDescriptor using the from_xml method with the same system, org, + and course """ - raise NotImplementedError('Modules must implement export_to_xml to enable xml export') + raise NotImplementedError( + 'Modules must implement export_to_xml to enable xml export') - # =============================== Testing =================================== + # =============================== Testing ================================== def get_sample_state(self): """ - Return a list of tuples of instance_state, shared_state. Each tuple defines a sample case for this module + Return a list of tuples of instance_state, shared_state. Each tuple + defines a sample case for this module """ return [('{}', '{}')] - # =============================== BUILTIN METHODS =========================== + # =============================== BUILTIN METHODS ========================== def __eq__(self, other): eq = (self.__class__ == other.__class__ and all(getattr(self, attr, None) == getattr(other, attr, None) @@ -493,38 +506,76 @@ class XModuleDescriptor(Plugin, HTMLSnippet): if not eq: for attr in self.equality_attributes: - print getattr(self, attr, None), getattr(other, attr, None), getattr(self, attr, None) == getattr(other, attr, None) + print(getattr(self, attr, None), + getattr(other, attr, None), + getattr(self, attr, None) == getattr(other, attr, None)) return eq def __repr__(self): - return "{class_}({system!r}, {definition!r}, location={location!r}, metadata={metadata!r})".format( + return ("{class_}({system!r}, {definition!r}, location={location!r}," + " metadata={metadata!r})".format( class_=self.__class__.__name__, system=self.system, definition=self.definition, location=self.location, metadata=self.metadata - ) + )) class DescriptorSystem(object): - def __init__(self, load_item, resources_fs, **kwargs): + def __init__(self, load_item, resources_fs, + error_handler, + **kwargs): """ load_item: Takes a Location and returns an XModuleDescriptor + resources_fs: A Filesystem object that contains all of the resources needed for the course + + error_handler: A hook for handling errors in loading the descriptor. + Must be a function of (error_msg, exc_info=None). + See errorhandlers.py for some simple ones. + + Patterns for using the error handler: + try: + x = access_some_resource() + check_some_format(x) + except SomeProblem: + msg = 'Grommet {0} is broken'.format(x) + log.exception(msg) # don't rely on handler to log + self.system.error_handler(msg) + # if we get here, work around if possible + raise # if no way to work around + OR + return 'Oops, couldn't load grommet' + + OR, if not in an exception context: + + if not check_something(thingy): + msg = "thingy {0} is broken".format(thingy) + log.critical(msg) + error_handler(msg) + # if we get here, work around + pass # e.g. if no workaround needed """ self.load_item = load_item self.resources_fs = resources_fs + self.error_handler = error_handler class XMLParsingSystem(DescriptorSystem): def __init__(self, load_item, resources_fs, process_xml, **kwargs): """ - process_xml: Takes an xml string, and returns the the XModuleDescriptor created from that xml + load_item: Takes a Location and returns an XModuleDescriptor + + process_xml: Takes an xml string, and returns a XModuleDescriptor + created from that xml + + """ - DescriptorSystem.__init__(self, load_item, resources_fs) + DescriptorSystem.__init__(self, load_item, resources_fs, **kwargs) self.process_xml = process_xml diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 49e0f79976..0b627ad4ee 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -187,7 +187,10 @@ class XmlDescriptor(XModuleDescriptor): with system.resources_fs.open(filepath) as file: definition_xml = cls.file_to_xml(file) except (ResourceNotFoundError, etree.XMLSyntaxError): - log.exception('Unable to load file contents at path %s' % filepath) + msg = 'Unable to load file contents at path %s' % filepath + log.exception(msg) + system.error_handler(msg) + # if error_handler didn't reraise, work around it. return {'data': 'Error loading file contents at path %s' % filepath} cls.clean_metadata_from_xml(definition_xml) @@ -206,20 +209,24 @@ class XmlDescriptor(XModuleDescriptor): @classmethod def _format_filepath(cls, category, name): - return u'{category}/{name}.{ext}'.format(category=category, name=name, ext=cls.filename_extension) + return u'{category}/{name}.{ext}'.format(category=category, + name=name, + ext=cls.filename_extension) def export_to_xml(self, resource_fs): """ - Returns an xml string representing this module, and all modules underneath it. - May also write required resources out to resource_fs + Returns an xml string representing this module, and all modules + underneath it. May also write required resources out to resource_fs - Assumes that modules have single parantage (that no module appears twice in the same course), - and that it is thus safe to nest modules as xml children as appropriate. + Assumes that modules have single parentage (that no module appears twice + in the same course), and that it is thus safe to nest modules as xml + children as appropriate. - The returned XML should be able to be parsed back into an identical XModuleDescriptor - using the from_xml method with the same system, org, and course + The returned XML should be able to be parsed back into an identical + XModuleDescriptor using the from_xml method with the same system, org, + and course - resource_fs is a pyfilesystem office (from the fs package) + resource_fs is a pyfilesystem object (from the fs package) """ xml_object = self.definition_to_xml(resource_fs) self.__class__.clean_metadata_from_xml(xml_object) @@ -244,7 +251,8 @@ class XmlDescriptor(XModuleDescriptor): attr_map = self.xml_attribute_map.get(attr, AttrMap(attr)) metadata_key = attr_map.metadata_key - if metadata_key not in self.metadata or metadata_key in self._inherited_metadata: + if (metadata_key not in self.metadata or + metadata_key in self._inherited_metadata): continue val = attr_map.from_metadata(self.metadata[metadata_key]) From e0513f5f244eee289b615230bdcdf65a14e06130 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:07:12 -0400 Subject: [PATCH 05/55] Fix course tag name on export of definition --- common/lib/xmodule/xmodule/xml_module.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 0b627ad4ee..9cf744461d 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -231,6 +231,8 @@ class XmlDescriptor(XModuleDescriptor): xml_object = self.definition_to_xml(resource_fs) self.__class__.clean_metadata_from_xml(xml_object) + xml_object.tag = self.category + xml_object.set('slug', self.name) # Put content in a separate file if it's large (has more than 5 descendent tags) if len(list(xml_object.iter())) > 5: @@ -244,9 +246,6 @@ class XmlDescriptor(XModuleDescriptor): xml_object.set('filename', self.name) - xml_object.set('slug', self.name) - xml_object.tag = self.category - for attr in self.metadata_attributes: attr_map = self.xml_attribute_map.get(attr, AttrMap(attr)) metadata_key = attr_map.metadata_key From 94ac61ffe2121c67acb61e6e82c73faa518f9f9a Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:08:53 -0400 Subject: [PATCH 06/55] Make decision about export file splitting overrideable * create split_to_file() function * chapters, html, problems always split * course never split * others based on size of subtree for now --- common/lib/xmodule/xmodule/capa_module.py | 5 +++++ common/lib/xmodule/xmodule/html_module.py | 5 +++++ common/lib/xmodule/xmodule/seq_module.py | 14 ++++++++++++++ common/lib/xmodule/xmodule/xml_module.py | 20 +++++++++++++++++--- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 5ff0c13198..735f664b0a 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -550,6 +550,7 @@ class CapaDescriptor(RawDescriptor): module_class = CapaModule + # VS[compat] # TODO (cpennington): Delete this method once all fall 2012 course are being # edited in the cms @classmethod @@ -558,3 +559,7 @@ class CapaDescriptor(RawDescriptor): 'problems/' + path[8:], path[8:], ] + @classmethod + def split_to_file(cls, xml_object): + '''Problems always written in their own files''' + return True diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 97509c6f34..ce18a05a8a 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -42,3 +42,8 @@ class HtmlDescriptor(RawDescriptor): def file_to_xml(cls, file_object): parser = etree.HTMLParser() return etree.parse(file_object, parser).getroot() + + @classmethod + def split_to_file(cls, xml_object): + # never include inline html + return True diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index d435be627b..5f7f41bf8d 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -121,3 +121,17 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): xml_object.append( etree.fromstring(child.export_to_xml(resource_fs))) return xml_object + + @classmethod + def split_to_file(cls, xml_object): + # Note: if we end up needing subclasses, can port this logic there. + yes = ('chapter',) + no = ('course',) + + if xml_object.tag in yes: + return True + elif xml_object.tag in no: + return False + + # otherwise maybe--delegate to superclass. + return XmlDescriptor.split_to_file(xml_object) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 9cf744461d..ee0b541de5 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -213,6 +213,20 @@ class XmlDescriptor(XModuleDescriptor): name=name, ext=cls.filename_extension) + @classmethod + def split_to_file(cls, xml_object): + ''' + Decide whether to write this object to a separate file or not. + + xml_object: an xml definition of an instance of cls. + + This default implementation will split if this has more than 7 + descendant tags. + + Can be overridden by subclasses. + ''' + return len(list(xml_object.iter())) > 7 + def export_to_xml(self, resource_fs): """ Returns an xml string representing this module, and all modules @@ -233,14 +247,14 @@ class XmlDescriptor(XModuleDescriptor): xml_object.tag = self.category xml_object.set('slug', self.name) - # Put content in a separate file if it's large (has more than 5 descendent tags) - if len(list(xml_object.iter())) > 5: + if self.split_to_file(xml_object): + # Put this object in it's own file filepath = self.__class__._format_filepath(self.category, self.name) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: file.write(etree.tostring(xml_object, pretty_print=True)) - + # ...and remove all of its children here for child in xml_object: xml_object.remove(child) From 9461148b6e14224ac919e3dca91f88d22bb0f759 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:09:47 -0400 Subject: [PATCH 07/55] make customtag take impl as attribute rather than child --- common/lib/xmodule/xmodule/backcompat_module.py | 3 +-- common/lib/xmodule/xmodule/template_module.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index 456a2c2b07..2b6ca5c25a 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -82,7 +82,6 @@ class TranslateCustomTagDescriptor(XModuleDescriptor): xml_object = etree.fromstring(xml_data) tag = xml_object.tag xml_object.tag = 'customtag' - impl = etree.SubElement(xml_object, 'impl') - impl.text = tag + xml_object.attrib['impl'] = tag return system.process_xml(etree.tostring(xml_object)) diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 268ce24559..3f926555f4 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -21,19 +21,23 @@ class CustomTagModule(XModule): course.xml:: ... - book + ... Renders to:: More information given in the text """ - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, + instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, + instance_state, shared_state, **kwargs) + xmltree = etree.fromstring(self.definition['data']) - template_name = xmltree.find('impl').text + template_name = xmltree.attrib['impl'] params = dict(xmltree.items()) - with self.system.filestore.open('custom_tags/{name}'.format(name=template_name)) as template: + with self.system.filestore.open( + 'custom_tags/{name}'.format(name=template_name)) as template: self.html = Template(template.read()).render(**params) def get_html(self): From 00da2b48e9eb6be23058adf3d6b14994dcdd339c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:10:21 -0400 Subject: [PATCH 08/55] make sure course "org" and "course" are preserved on export --- common/lib/xmodule/xmodule/course_module.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 05f440c0f8..0808d5fbb0 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -10,6 +10,7 @@ log = logging.getLogger(__name__) class CourseDescriptor(SequenceDescriptor): module_class = SequenceModule + metadata_attributes = SequenceDescriptor.metadata_attributes + ('org', 'course') def __init__(self, system, definition=None, **kwargs): super(CourseDescriptor, self).__init__(system, definition, **kwargs) From a7159716f9ef0a7da9d40867a0ee7dc8dd7c00b9 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:11:00 -0400 Subject: [PATCH 09/55] fix out-of-date code * no more __xmltree. --- common/lib/xmodule/xmodule/x_module.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 3e564053bd..12881a0434 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -3,6 +3,7 @@ import pkg_resources import logging from xmodule.modulestore import Location + from functools import partial log = logging.getLogger('mitx.' + __name__) @@ -192,11 +193,7 @@ class XModule(HTMLSnippet): self._loaded_children = None def get_name(self): - name = self.__xmltree.get('name') - if name: - return name - else: - raise "We should iterate through children and find a default name" + return self.name def get_children(self): ''' From beda4f95b26694cf17bcf8cf22a9e98db4392970 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:12:16 -0400 Subject: [PATCH 10/55] Initial version of import/check/export script. * uses xml modulestore, and new error_handler hook --- .../management/commands/clean_xml.py | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 lms/djangoapps/courseware/management/commands/clean_xml.py diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py new file mode 100644 index 0000000000..1803553ba3 --- /dev/null +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -0,0 +1,143 @@ +import sys +import traceback + +from fs.osfs import OSFS +from path import path +from lxml import etree + +from django.core.management.base import BaseCommand + +from xmodule.modulestore.xml import XMLModuleStore + + +def export(course, export_dir): + """Export the specified course to course_dir. Creates dir if it doesn't exist. + Overwrites files, does not clean out dir beforehand. + """ + fs = OSFS(export_dir, create=True) + if not fs.isdirempty('.'): + print ('WARNING: Directory {dir} not-empty.' + ' May clobber/confuse things'.format(dir=export_dir)) + + try: + xml = course.export_to_xml(fs) + with fs.open('course.xml', mode='w') as f: + f.write(xml) + + return True + except: + print 'Export failed!' + traceback.print_exc() + + return False + + +def traverse_tree(course): + '''Load every descriptor in course. Return bool success value.''' + queue = [course] + while len(queue) > 0: + node = queue.pop() +# print '{0}:'.format(node.location) +# if 'data' in node.definition: +# print '{0}'.format(node.definition['data']) + queue.extend(node.get_children()) + + return True + +def make_logging_error_handler(): + '''Return a tuple (handler, error_list), where + the handler appends the message and any exc_info + to the error_list on every call. + ''' + errors = [] + + def error_handler(msg, exc_info=None): + '''Log errors''' + if exc_info is None: + if sys.exc_info() != (None, None, None): + exc_info = sys.exc_info() + + errors.append((msg, exc_info)) + + return (error_handler, errors) + +def clean_xml(course_dir, export_dir, verbose=True): + all_ok = True + + print "Attempting to load '{0}'".format(course_dir) + + course_dir = path(course_dir) + data_dir = course_dir.dirname() + course_dirs = [course_dir.basename()] + + (error_handler, errors) = make_logging_error_handler() + # No default class--want to complain if it doesn't find plugins for any + # module. + modulestore = XMLModuleStore(data_dir, + default_class=None, + eager=True, + course_dirs=course_dirs, + error_handler=error_handler) + + def str_of_err(tpl): + (msg, exc_info) = tpl + if exc_info is None: + return msg + + exc_str = '\n'.join(traceback.format_exception(*exc_info)) + return '{msg}\n{exc}'.format(msg=msg, exc=exc_str) + + courses = modulestore.get_courses() + if len(errors) != 0: + all_ok = False + print '\n' + print "=" * 40 + print 'ERRORs during import:' + print '\n'.join(map(str_of_err,errors)) + print "=" * 40 + print '\n' + + n = len(courses) + if n != 1: + print 'ERROR: Expect exactly 1 course. Loaded {n}: {lst}'.format( + n=n, lst=courses) + return + + course = courses[0] + + print course + validators = ( + traverse_tree, + ) + + print "=" * 40 + print "Running validators..." + + for validate in validators: + print 'Running {0}'.format(validate.__name__) + all_ok = validate(course) and all_ok + + + if all_ok: + print 'Course passes all checks!' + export(course, export_dir) + + else: + print "Course fails some checks. See above for errors." + print "Did NOT export" + + + +class Command(BaseCommand): + help = """Imports specified course.xml, validate it, then exports in + a canonical format. + +Usage: clean_xml PATH-TO-COURSE-DIR PATH-TO-OUTPUT-DIR +""" + + def handle(self, *args, **options): + if len(args) != 2: + print Command.help + return + + clean_xml(args[0], args[1]) From a5bacb469622b1b7834b89be95a21c6bea09687c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 25 Jul 2012 14:16:52 -0400 Subject: [PATCH 11/55] add now-mandatory metadata to toy course.xml --- common/test/data/toy/course.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/test/data/toy/course.xml b/common/test/data/toy/course.xml index 645fbd7af8..ba4b295c2e 100644 --- a/common/test/data/toy/course.xml +++ b/common/test/data/toy/course.xml @@ -1,4 +1,4 @@ - + diff --git a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml index 28b56fb9b0..5c4c65f12d 100644 --- a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml +++ b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml @@ -2,17 +2,13 @@ - - discuss - + S1E4 has been removed. diff --git a/common/test/data/full/sequential/System_Usage_Sequence.xml b/common/test/data/full/sequential/System_Usage_Sequence.xml index baf73f1e9e..571229be7c 100644 --- a/common/test/data/full/sequential/System_Usage_Sequence.xml +++ b/common/test/data/full/sequential/System_Usage_Sequence.xml @@ -3,5 +3,4 @@ - diff --git a/common/test/data/full/vertical/vertical_58.xml b/common/test/data/full/vertical/vertical_58.xml index 7eba0f9dad..5707e6acf3 100644 --- a/common/test/data/full/vertical/vertical_58.xml +++ b/common/test/data/full/vertical/vertical_58.xml @@ -1,12 +1,6 @@ diff --git a/common/test/data/full/vertical/vertical_89.xml b/common/test/data/full/vertical/vertical_89.xml index 06b13846d2..da15a6751a 100644 --- a/common/test/data/full/vertical/vertical_89.xml +++ b/common/test/data/full/vertical/vertical_89.xml @@ -3,13 +3,7 @@