Clean up DescriptorSystem constructor hierarchy

In response to spending an hour with a strange argument passing bug that neither I nor Cale could figure out.

* pass all arguments explicitly
* pass arguments in a consistent order between classes

common/lib/xmodule/xmodule/mako_module.py

@@ -2,9 +2,12 @@ from x_module import XModuleDescriptor, DescriptorSystem
 
 
 class MakoDescriptorSystem(DescriptorSystem):
-    def __init__(self, render_template, *args, **kwargs):
+    def __init__(self, load_item, resources_fs, error_handler,
+                 render_template):
+        super(MakoDescriptorSystem, self).__init__(
+            load_item, resources_fs, error_handler)
+
         self.render_template = render_template
-        super(MakoDescriptorSystem, self).__init__(*args, **kwargs)
 
 
 class MakoModuleDescriptor(XModuleDescriptor):

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -6,6 +6,7 @@ from fs.osfs import OSFS
 from itertools import repeat
 
 from importlib import import_module
+from xmodule.errorhandlers import strict_error_handler
 from xmodule.x_module import XModuleDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from mitxmako.shortcuts import render_to_string
@@ -23,15 +24,26 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
     A system that has a cache of module json that it will use to load modules
     from, with a backup of calling to the underlying modulestore for more data
     """
-    def __init__(self, modulestore, module_data, default_class, resources_fs, render_template):
+    def __init__(self, modulestore, module_data, default_class, resources_fs,
+                 error_handler, render_template):
         """
         modulestore: the module store that can be used to retrieve additional modules
-        module_data: a dict mapping Location -> json that was cached from the underlying modulestore
-        default_class: The default_class to use when loading an XModuleDescriptor from the module_data
+
+        module_data: a dict mapping Location -> json that was cached from the
+            underlying modulestore
+
+        default_class: The default_class to use when loading an
+            XModuleDescriptor from the module_data
+
         resources_fs: a filesystem, as per MakoDescriptorSystem
-        render_template: a function for rendering templates, as per MakoDescriptorSystem
+
+        error_handler:
+
+        render_template: a function for rendering templates, as per
+            MakoDescriptorSystem
         """
-        super(CachingDescriptorSystem, self).__init__(render_template, self.load_item, resources_fs)
+        super(CachingDescriptorSystem, self).__init__(
+                self.load_item, resources_fs, error_handler, render_template)
         self.modulestore = modulestore
         self.module_data = module_data
         self.default_class = default_class
@@ -127,13 +139,15 @@ class MongoModuleStore(ModuleStore):
         """
         Load an XModuleDescriptor from item, using the children stored in data_cache
         """
-        resource_fs = OSFS(self.fs_root / item.get('data_dir', item['location']['course']))
+        resource_fs = OSFS(self.fs_root / item.get('data_dir',
+                                                   item['location']['course']))
         system = CachingDescriptorSystem(
             self,
             data_cache,
             self.default_class,
             resource_fs,
-            render_to_string
+            strict_error_handler,
+            render_to_string,
         )
         return system.load_item(item['location'])
 

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -13,8 +13,9 @@ import re
 from . import ModuleStore, Location
 from .exceptions import ItemNotFoundError
 
-etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False,
-                                         remove_comments=True, remove_blank_text=True))
+etree.set_default_parser(
+    etree.XMLParser(dtd_validation=False, load_dtd=False,
+                    remove_comments=True, remove_blank_text=True))
 
 log = logging.getLogger('mitx.' + __name__)
 
@@ -28,8 +29,7 @@ def clean_out_mako_templating(xml_string):
     return xml_string
 
 class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
-    def __init__(self, xmlstore, org, course, course_dir,
-                 error_handler):
+    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.
@@ -82,15 +82,14 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
                 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)
+        render_template = lambda: ''
+        load_item = xmlstore.get_item
+        resources_fs = OSFS(xmlstore.data_dir / course_dir)
+
+        MakoDescriptorSystem.__init__(self, load_item, resources_fs,
+                                      error_handler, render_template)
+        XMLParsingSystem.__init__(self, load_item, resources_fs,
+                                  error_handler, process_xml)
 
 
 

common/lib/xmodule/xmodule/x_module.py

@@ -521,9 +521,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
 
 
 class DescriptorSystem(object):
-    def __init__(self, load_item, resources_fs,
-                 error_handler,
-                 **kwargs):
+    def __init__(self, load_item, resources_fs, error_handler):
         """
         load_item: Takes a Location and returns an XModuleDescriptor
 
@@ -563,16 +561,14 @@ class DescriptorSystem(object):
 
 
 class XMLParsingSystem(DescriptorSystem):
-    def __init__(self, load_item, resources_fs, process_xml, **kwargs):
+    def __init__(self, load_item, resources_fs, error_handler, process_xml):
         """
-        load_item: Takes a Location and returns an XModuleDescriptor
+        load_item, resources_fs, error_handler: see DescriptorSystem
 
         process_xml: Takes an xml string, and returns a XModuleDescriptor
             created from that xml
-
-
         """
-        DescriptorSystem.__init__(self, load_item, resources_fs, **kwargs)
+        DescriptorSystem.__init__(self, load_item, resources_fs, error_handler)
         self.process_xml = process_xml