diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 5e39ec93d1..64392b7643 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -16,8 +16,9 @@ from .store_utilities import rewrite_nonportable_content_links log = logging.getLogger(__name__) -def import_static_content(modules, course_loc, course_data_path, static_content_store, target_location_namespace, - subpath='static', verbose=False): +def import_static_content( + modules, course_loc, course_data_path, static_content_store, + target_location_namespace, subpath='static', verbose=False): remap_dict = {} @@ -27,8 +28,8 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ with open(course_data_path / 'policies/assets.json') as f: policy = json.load(f) except (IOError, ValueError) as err: - # xml backed courses won't have this file, only exported courses; so, its absence is not - # really an exception. + # xml backed courses won't have this file, only exported courses; + # so, its absence is not really an exception. policy = {} verbose = True @@ -45,49 +46,60 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ data = f.read() except IOError: if filename.startswith('._'): - # OS X "companion files". See http://www.diigo.com/annotated/0c936fda5da4aa1159c189cea227e174 + # OS X "companion files". See + # http://www.diigo.com/annotated/0c936fda5da4aa1159c189cea227e174 continue # Not a 'hidden file', then re-raise exception raise - fullname_with_subpath = content_path.replace(static_dir, '') # strip away leading path from the name + # strip away leading path from the name + fullname_with_subpath = content_path.replace(static_dir, '') if fullname_with_subpath.startswith('/'): fullname_with_subpath = fullname_with_subpath[1:] - content_loc = StaticContent.compute_location(target_location_namespace.org, target_location_namespace.course, fullname_with_subpath) - + content_loc = StaticContent.compute_location( + target_location_namespace.org, target_location_namespace.course, + fullname_with_subpath + ) policy_ele = policy.get(content_loc.name, {}) displayname = policy_ele.get('displayname', filename) locked = policy_ele.get('locked', False) - mime_type = policy_ele.get('contentType', mimetypes.guess_type(filename)[0]) + mime_type = policy_ele.get( + 'contentType', + mimetypes.guess_type(filename)[0] + ) content = StaticContent( content_loc, displayname, mime_type, data, import_path=fullname_with_subpath, locked=locked ) # first let's save a thumbnail so we can get back a thumbnail location - (thumbnail_content, thumbnail_location) = static_content_store.generate_thumbnail(content) + thumbnail_content, thumbnail_location = static_content_store.generate_thumbnail(content) if thumbnail_content is not None: content.thumbnail_location = thumbnail_location - #then commit the content + # then commit the content try: static_content_store.save(content) except Exception as err: - log.exception('Error importing {0}, error={1}'.format(fullname_with_subpath, err)) + log.exception('Error importing {0}, error={1}'.format( + fullname_with_subpath, err + )) - #store the remapping information which will be needed to subsitute in the module data + # store the remapping information which will be needed + # to subsitute in the module data remap_dict[fullname_with_subpath] = content_loc.name return remap_dict -def import_from_xml(store, data_dir, course_dirs=None, - default_class='xmodule.raw_module.RawDescriptor', - load_error_modules=True, static_content_store=None, target_location_namespace=None, - verbose=False, draft_store=None, - do_import_static=True): +def import_from_xml( + store, data_dir, course_dirs=None, + default_class='xmodule.raw_module.RawDescriptor', + load_error_modules=True, static_content_store=None, + target_location_namespace=None, verbose=False, draft_store=None, + do_import_static=True): """ Import the specified xml data_dir into the "store" modulestore, using org and course as the location org and course. @@ -95,14 +107,20 @@ def import_from_xml(store, data_dir, course_dirs=None, course_dirs: If specified, the list of course_dirs to load. Otherwise, load all course dirs - target_location_namespace is the namespace [passed as Location] (i.e. {tag},{org},{course}) that all modules in the should be remapped to - after import off disk. We do this remapping as a post-processing step because there's logic in the importing which - expects a 'url_name' as an identifier to where things are on disk e.g. ../policies//policy.json as well as metadata keys in + target_location_namespace is the namespace [passed as Location] + (i.e. {tag},{org},{course}) that all modules in the should be remapped to + after import off disk. We do this remapping as a post-processing step + because there's logic in the importing which expects a 'url_name' as an + identifier to where things are on disk + e.g. ../policies//policy.json as well as metadata keys in the policy.json. so we need to keep the original url_name during import - do_import_static: if False, then static files are not imported into the static content store. This can be employed for courses which - have substantial unchanging static content, which is to inefficient to import every time the course is loaded. - Static content for some courses may also be served directly by nginx, instead of going through django. + :param do_import_static: + if False, then static files are not imported into the static content + store. This can be employed for courses which have substantial + unchanging static content, which is to inefficient to import every + time the course is loaded. Static content for some courses may also be + served directly by nginx, instead of going through django. """ @@ -114,21 +132,26 @@ def import_from_xml(store, data_dir, course_dirs=None, xblock_mixins=store.xblock_mixins, ) - # NOTE: the XmlModuleStore does not implement get_items() which would be a preferable means - # to enumerate the entire collection of course modules. It will be left as a TBD to implement that + # NOTE: the XmlModuleStore does not implement get_items() + # which would be a preferable means to enumerate the entire collection + # of course modules. It will be left as a TBD to implement that # method on XmlModuleStore. course_items = [] for course_id in xml_module_store.modules.keys(): if target_location_namespace is not None: - pseudo_course_id = '/'.join([target_location_namespace.org, target_location_namespace.course]) + pseudo_course_id = '/'.join( + [target_location_namespace.org, target_location_namespace.course] + ) else: course_id_components = course_id.split('/') - pseudo_course_id = '/'.join([course_id_components[0], course_id_components[1]]) + pseudo_course_id = '/'.join( + [course_id_components[0], course_id_components[1]] + ) try: - # turn off all write signalling while importing as this is a high volume operation - # on stores that need it + # turn off all write signalling while importing as this + # is a high volume operation on stores that need it if (hasattr(store, 'ignore_write_events_on_courses') and pseudo_course_id not in store.ignore_write_events_on_courses): store.ignore_write_events_on_courses.append(pseudo_course_id) @@ -139,69 +162,104 @@ def import_from_xml(store, data_dir, course_dirs=None, if verbose: log.debug("Scanning {0} for course module...".format(course_id)) - # Quick scan to get course module as we need some info from there. Also we need to make sure that the - # course module is committed first into the store + # Quick scan to get course module as we need some info from there. + # Also we need to make sure that the course module is committed + # first into the store for module in xml_module_store.modules[course_id].itervalues(): if module.scope_ids.block_type == 'course': course_data_path = path(data_dir) / module.data_dir course_location = module.location - log.debug('======> IMPORTING course to location {0}'.format(course_location)) + log.debug('======> IMPORTING course to location {loc}'.format( + loc=course_location + )) module = remap_namespace(module, target_location_namespace) if not do_import_static: - module.static_asset_path = module.data_dir # for old-style xblock where this was actually linked to kvs + # for old-style xblock where this was actually linked to kvs + module.static_asset_path = module.data_dir module.save() - log.debug('course static_asset_path={0}'.format(module.static_asset_path)) + log.debug('course static_asset_path={path}'.format( + path=module.static_asset_path + )) log.debug('course data_dir={0}'.format(module.data_dir)) - # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which - # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS, - # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that - - # if there is *any* tabs - then there at least needs to be some predefined ones + # cdodge: more hacks (what else). Seems like we have a + # problem when importing a course (like 6.002) which + # does not have any tabs defined in the policy file. + # The import goes fine and then displays fine in LMS, + # but if someone tries to add a new tab in the CMS, then + # the LMS barfs because it expects that -- if there are + # *any* tabs -- then there at least needs to be + # some predefined ones if module.tabs is None or len(module.tabs) == 0: - module.tabs = [{"type": "courseware"}, - {"type": "course_info", "name": "Course Info"}, - {"type": "discussion", "name": "Discussion"}, - {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge + module.tabs = [ + {"type": "courseware"}, + {"type": "course_info", "name": "Course Info"}, + {"type": "discussion", "name": "Discussion"}, + {"type": "wiki", "name": "Wiki"}, + # note, add 'progress' when we can support it on Edge + ] - import_module(module, store, course_data_path, static_content_store, course_location, - target_location_namespace or course_location, do_import_static=do_import_static) + import_module( + module, store, course_data_path, static_content_store, + course_location, + target_location_namespace or course_location, + do_import_static=do_import_static + ) course_items.append(module) # then import all the static content if static_content_store is not None and do_import_static: - _namespace_rename = target_location_namespace if target_location_namespace is not None else course_location + if target_location_namespace is not None: + _namespace_rename = target_location_namespace + else: + _namespace_rename = course_location # first pass to find everything in /static/ - import_static_content(xml_module_store.modules[course_id], course_location, course_data_path, static_content_store, - _namespace_rename, subpath='static', verbose=verbose) + import_static_content( + xml_module_store.modules[course_id], course_location, + course_data_path, static_content_store, + _namespace_rename, subpath='static', verbose=verbose + ) elif verbose and not do_import_static: - log.debug('Skipping import of static content, since do_import_static={0}'.format(do_import_static)) + log.debug( + "Skipping import of static content, " + "since do_import_static={0}".format(do_import_static) + ) # no matter what do_import_static is, import "static_import" directory - # This is needed because the "about" pages (eg "overview") are loaded via load_extra_content, and - # do not inherit the lms metadata from the course module, and thus do not get "static_content_store" - # properly defined. Static content referenced in those extra pages thus need to come through the - # c4x:// contentstore, unfortunately. Tell users to copy that content into the "static_import" subdir. + # This is needed because the "about" pages (eg "overview") are + # loaded via load_extra_content, and do not inherit the lms + # metadata from the course module, and thus do not get + # "static_content_store" properly defined. Static content + # referenced in those extra pages thus need to come through the + # c4x:// contentstore, unfortunately. Tell users to copy that + # content into the "static_import" subdir. simport = 'static_import' if os.path.exists(course_data_path / simport): - _namespace_rename = target_location_namespace if target_location_namespace is not None else course_location + if target_location_namespace is not None: + _namespace_rename = target_location_namespace + else: + _namespace_rename = course_location - import_static_content(xml_module_store.modules[course_id], course_location, course_data_path, static_content_store, - _namespace_rename, subpath=simport, verbose=verbose) + import_static_content( + xml_module_store.modules[course_id], course_location, + course_data_path, static_content_store, + _namespace_rename, subpath=simport, verbose=verbose + ) # finally loop through all the modules for module in xml_module_store.modules[course_id].itervalues(): if module.scope_ids.block_type == 'course': - # we've already saved the course module up at the top of the loop - # so just skip over it in the inner loop + # we've already saved the course module up at the top + # of the loop so just skip over it in the inner loop continue # remap module to the new namespace @@ -209,11 +267,16 @@ def import_from_xml(store, data_dir, course_dirs=None, module = remap_namespace(module, target_location_namespace) if verbose: - log.debug('importing module location {0}'.format(module.location)) + log.debug('importing module location {loc}'.format( + loc=module.location + )) - import_module(module, store, course_data_path, static_content_store, course_location, - target_location_namespace if target_location_namespace else course_location, - do_import_static=do_import_static) + import_module( + module, store, course_data_path, static_content_store, + course_location, + target_location_namespace if target_location_namespace else course_location, + do_import_static=do_import_static + ) # now import any 'draft' items if draft_store is not None: @@ -239,11 +302,14 @@ def import_from_xml(store, data_dir, course_dirs=None, return xml_module_store, course_items -def import_module(module, store, course_data_path, static_content_store, - source_course_location, dest_course_location, allow_not_found=False, - do_import_static=True): +def import_module( + module, store, course_data_path, static_content_store, + source_course_location, dest_course_location, allow_not_found=False, + do_import_static=True): - logging.debug('processing import of module {0}...'.format(module.location.url())) + logging.debug('processing import of module {url}...'.format( + url=module.location.url() + )) content = {} for field in module.fields.values(): @@ -262,13 +328,17 @@ def import_module(module, store, course_data_path, static_content_store, module_data = content if isinstance(module_data, basestring) and do_import_static: - # we want to convert all 'non-portable' links in the module_data (if it is a string) to - # portable strings (e.g. /static/) + # we want to convert all 'non-portable' links in the module_data + # (if it is a string) to portable strings (e.g. /static/) module_data = rewrite_nonportable_content_links( - source_course_location.course_id, dest_course_location.course_id, module_data) + source_course_location.course_id, + dest_course_location.course_id, module_data + ) if allow_not_found: - store.update_item(module.location, module_data, allow_not_found=allow_not_found) + store.update_item( + module.location, module_data, allow_not_found=allow_not_found + ) else: store.update_item(module.location, module_data) @@ -278,23 +348,29 @@ def import_module(module, store, course_data_path, static_content_store, # NOTE: It's important to use own_metadata here to avoid writing # inherited metadata everywhere. - # remove any export/import only xml_attributes which are used to wire together draft imports - if hasattr(module, 'xml_attributes') and 'parent_sequential_url' in module.xml_attributes: + # remove any export/import only xml_attributes + # which are used to wire together draft imports + if 'parent_sequential_url' in getattr(module, 'xml_attributes', []): del module.xml_attributes['parent_sequential_url'] - if hasattr(module, 'xml_attributes') and 'index_in_children_list' in module.xml_attributes: + if 'index_in_children_list' in getattr(module, 'xml_attributes', []): del module.xml_attributes['index_in_children_list'] module.save() store.update_metadata(module.location, dict(own_metadata(module))) -def import_course_draft(xml_module_store, store, draft_store, course_data_path, static_content_store, source_location_namespace, target_location_namespace): +def import_course_draft( + xml_module_store, store, draft_store, course_data_path, + static_content_store, source_location_namespace, + target_location_namespace): ''' This will import all the content inside of the 'drafts' folder, if it exists - NOTE: This is not a full course import, basically in our current application only verticals (and downwards) - can be in draft. Therefore, we need to use slightly different call points into the import process_xml - as we can't simply call XMLModuleStore() constructor (like we do for importing public content) + NOTE: This is not a full course import, basically in our current + application only verticals (and downwards) can be in draft. + Therefore, we need to use slightly different call points into + the import process_xml as we can't simply call XMLModuleStore() constructor + (like we do for importing public content) ''' draft_dir = course_data_path + "/drafts" if not os.path.exists(draft_dir): @@ -312,36 +388,42 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path, load_error_modules=False, ) - # now walk the /vertical directory where each file in there will be a draft copy of the Vertical + # now walk the /vertical directory where each file in there + # will be a draft copy of the Vertical for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"): for filename in filenames: module_path = os.path.join(dirname, filename) with open(module_path, 'r') as f: try: - # note, on local dev it seems like OSX will put some extra files in - # the directory with "quarantine" information. These files are - # binary files and will throw exceptions when we try to parse - # the file as an XML string. Let's make sure we're + # note, on local dev it seems like OSX will put + # some extra files in the directory with "quarantine" + # information. These files are binary files and will + # throw exceptions when we try to parse the file + # as an XML string. Let's make sure we're # dealing with a string before ingesting data = f.read() try: xml = data.decode('utf-8') except UnicodeDecodeError, err: - # seems like on OSX localdev, the OS is making quarantine files - # in the unzip directory when importing courses - # so if we blindly try to enumerate through the directory, we'll try - # to process a bunch of binary quarantine files (which are prefixed with a '._' character - # which will dump a bunch of exceptions to the output, although they are harmless. + # seems like on OSX localdev, the OS is making + # quarantine files in the unzip directory + # when importing courses so if we blindly try to + # enumerate through the directory, we'll try + # to process a bunch of binary quarantine files + # (which are prefixed with a '._' character which + # will dump a bunch of exceptions to the output, + # although they are harmless. # - # Reading online docs there doesn't seem to be a good means to detect a 'hidden' - # file that works well across all OS environments. So for now, I'm using - # OSX's utilization of a leading '.' in the filename to indicate a system hidden - # file. - # - # Better yet would be a way to figure out if this is a binary file, but I - # haven't found a good way to do this yet. + # Reading online docs there doesn't seem to be + # a good means to detect a 'hidden' file that works + # well across all OS environments. So for now, I'm using + # OSX's utilization of a leading '.' in the filename + # to indicate a system hidden file. # + # Better yet would be a way to figure out if this is + # a binary file, but I haven't found a good way + # to do this yet. if filename.startswith('._'): continue # Not a 'hidden file', then re-raise exception @@ -352,8 +434,9 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path, def _import_module(module): module.location = module.location._replace(revision='draft') # make sure our parent has us in its list of children - # this is to make sure private only verticals show up in the list of children since - # they would have been filtered out from the non-draft store export + # this is to make sure private only verticals show up + # in the list of children since they would have been + # filtered out from the non-draft store export if module.location.category == 'vertical': non_draft_location = module.location._replace(revision=None) sequential_url = module.xml_attributes['parent_sequential_url'] @@ -361,41 +444,48 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path, seq_location = Location(sequential_url) - # IMPORTANT: Be sure to update the sequential in the NEW namespace - seq_location = seq_location._replace(org=target_location_namespace.org, - course=target_location_namespace.course - ) + # IMPORTANT: Be sure to update the sequential + # in the NEW namespace + seq_location = seq_location._replace( + org=target_location_namespace.org, + course=target_location_namespace.course + ) sequential = store.get_item(seq_location, depth=0) if non_draft_location.url() not in sequential.children: sequential.children.insert(index, non_draft_location.url()) store.update_children(sequential.location, sequential.children) - import_module(module, draft_store, course_data_path, static_content_store, - source_location_namespace, target_location_namespace, allow_not_found=True) + import_module( + module, draft_store, course_data_path, + static_content_store, source_location_namespace, + target_location_namespace, allow_not_found=True + ) for child in module.get_children(): _import_module(child) # HACK: since we are doing partial imports of drafts - # the vertical doesn't have the 'url-name' set in the attributes (they are normally in the parent - # object, aka sequential), so we have to replace the location.name with the XML filename - # that is part of the pack + # the vertical doesn't have the 'url-name' set in the + # attributes (they are normally in the parent object, + # aka sequential), so we have to replace the location.name + # with the XML filename that is part of the pack fn, fileExtension = os.path.splitext(filename) descriptor.location = descriptor.location._replace(name=fn) _import_module(descriptor) except Exception, e: - logging.exception('There was an error. {0}'.format(unicode(e))) - pass + logging.exception('There was an error. {err}'.format( + err=unicode(e) + )) def remap_namespace(module, target_location_namespace): if target_location_namespace is None: return module - # This looks a bit wonky as we need to also change the 'name' of the imported course to be what - # the caller passed in + # This looks a bit wonky as we need to also change the 'name' of the + # imported course to be what the caller passed in if module.location.category != 'course': module.location = module.location._replace( tag=target_location_namespace.tag, @@ -413,18 +503,20 @@ def remap_namespace(module, target_location_namespace): course=target_location_namespace.course, name=target_location_namespace.name ) - # - # There is more re-namespacing work we have to do when importing course modules - # + # There is more re-namespacing work we have to do when + # importing course modules # remap pdf_textbook urls for entry in module.pdf_textbooks: for chapter in entry.get('chapters', []): if StaticContent.is_c4x_path(chapter.get('url', '')): - chapter['url'] = StaticContent.renamespace_c4x_path(chapter['url'], target_location_namespace) + chapter['url'] = StaticContent.renamespace_c4x_path( + chapter['url'], target_location_namespace + ) - # if there is a wiki_slug which is the same as the original location (aka default value), - # then remap that so the wiki doesn't point to the old Wiki. + # if there is a wiki_slug which is the same as the original location + # (aka default value), then remap that so the wiki doesn't point to + # the old Wiki. if module.wiki_slug == original_location.course: module.wiki_slug = target_location_namespace.course @@ -461,8 +553,9 @@ def allowed_metadata_by_category(category): def check_module_metadata_editability(module): ''' - Assert that there is no metadata within a particular module that we can't support editing - However we always allow 'display_name' and 'xml_attribtues' + Assert that there is no metadata within a particular module that + we can't support editing. However we always allow 'display_name' + and 'xml_attributes' ''' allowed = allowed_metadata_by_category(module.location.category) if '*' in allowed: @@ -475,7 +568,12 @@ def check_module_metadata_editability(module): if len(illegal_keys) > 0: err_cnt = err_cnt + 1 - print ': found non-editable metadata on {0}. These metadata keys are not supported = {1}'. format(module.location.url(), illegal_keys) + print( + ": found non-editable metadata on {url}. " + "These metadata keys are not supported = {keys}".format( + url=module.location.url(), keys=illegal_keys + ) + ) return err_cnt @@ -490,7 +588,8 @@ def validate_no_non_editable_metadata(module_store, course_id, category): return err_cnt -def validate_category_hierarchy(module_store, course_id, parent_category, expected_child_category): +def validate_category_hierarchy( + module_store, course_id, parent_category, expected_child_category): err_cnt = 0 parents = [] @@ -503,8 +602,14 @@ def validate_category_hierarchy(module_store, course_id, parent_category, expect for child_loc in [Location(child) for child in parent.children]: if child_loc.category != expected_child_category: err_cnt += 1 - print 'ERROR: child {0} of parent {1} was expected to be category of {2} but was {3}'.format( - child_loc, parent.location, expected_child_category, child_loc.category) + print( + "ERROR: child {child} of parent {parent} was expected to be " + "category of {expected} but was {actual}".format( + child=child_loc, parent=parent.location, + expected=expected_child_category, + actual=child_loc.category + ) + ) return err_cnt @@ -512,8 +617,13 @@ def validate_category_hierarchy(module_store, course_id, parent_category, expect def validate_data_source_path_existence(path, is_err=True, extra_msg=None): _cnt = 0 if not os.path.exists(path): - print ("{0}: Expected folder at {1}. {2}".format('ERROR' if is_err == True else 'WARNING', path, extra_msg if - extra_msg is not None else '')) + print( + "{type}: Expected folder at {path}. {extra}".format( + type='ERROR' if is_err else 'WARNING', + path=path, + extra=extra_msg or "", + ) + ) _cnt = 1 return _cnt @@ -524,15 +634,17 @@ def validate_data_source_paths(data_dir, course_dir): err_cnt = 0 warn_cnt = 0 err_cnt += validate_data_source_path_existence(course_path / 'static') - warn_cnt += validate_data_source_path_existence(course_path / 'static/subs', is_err=False, - extra_msg='Video captions (if they are used) will not work unless they are static/subs.') + warn_cnt += validate_data_source_path_existence( + course_path / 'static/subs', is_err=False, + extra_msg='Video captions (if they are used) will not work unless they are static/subs.' + ) return err_cnt, warn_cnt def validate_course_policy(module_store, course_id): """ - Validate that the course explicitly sets values for any fields whose defaults may have changed between - the export and the import. + Validate that the course explicitly sets values for any fields + whose defaults may have changed between the export and the import. Does not add to error count as these are just warnings. """ @@ -542,16 +654,25 @@ def validate_course_policy(module_store, course_id): if module.location.category == 'course': if not module._field_data.has(module, 'rerandomize'): warn_cnt += 1 - print 'WARN: course policy does not specify value for "rerandomize" whose default is now "never". The behavior of your course may change.' + print( + 'WARN: course policy does not specify value for ' + '"rerandomize" whose default is now "never". ' + 'The behavior of your course may change.' + ) if not module._field_data.has(module, 'showanswer'): warn_cnt += 1 - print 'WARN: course policy does not specify value for "showanswer" whose default is now "finished". The behavior of your course may change.' + print( + 'WARN: course policy does not specify value for ' + '"showanswer" whose default is now "finished". ' + 'The behavior of your course may change.' + ) return warn_cnt -def perform_xlint(data_dir, course_dirs, - default_class='xmodule.raw_module.RawDescriptor', - load_error_modules=True): +def perform_xlint( + data_dir, course_dirs, + default_class='xmodule.raw_module.RawDescriptor', + load_error_modules=True): err_cnt = 0 warn_cnt = 0 @@ -581,7 +702,7 @@ def perform_xlint(data_dir, course_dirs, for err_log in module_store.errored_courses.itervalues(): for err_log_entry in err_log.errors: msg = err_log_entry[0] - print msg + print(msg) if msg.startswith('ERROR:'): err_cnt += 1 else: @@ -589,33 +710,64 @@ def perform_xlint(data_dir, course_dirs, for course_id in module_store.modules.keys(): # constrain that courses only have 'chapter' children - err_cnt += validate_category_hierarchy(module_store, course_id, "course", "chapter") + err_cnt += validate_category_hierarchy( + module_store, course_id, "course", "chapter" + ) # constrain that chapters only have 'sequentials' - err_cnt += validate_category_hierarchy(module_store, course_id, "chapter", "sequential") + err_cnt += validate_category_hierarchy( + module_store, course_id, "chapter", "sequential" + ) # constrain that sequentials only have 'verticals' - err_cnt += validate_category_hierarchy(module_store, course_id, "sequential", "vertical") - # validate the course policy overrides any defaults which have changed over time + err_cnt += validate_category_hierarchy( + module_store, course_id, "sequential", "vertical" + ) + # validate the course policy overrides any defaults + # which have changed over time warn_cnt += validate_course_policy(module_store, course_id) # don't allow metadata on verticals, since we can't edit them in studio - err_cnt += validate_no_non_editable_metadata(module_store, course_id, "vertical") + err_cnt += validate_no_non_editable_metadata( + module_store, course_id, "vertical" + ) # don't allow metadata on chapters, since we can't edit them in studio - err_cnt += validate_no_non_editable_metadata(module_store, course_id, "chapter") + err_cnt += validate_no_non_editable_metadata( + module_store, course_id, "chapter" + ) # don't allow metadata on sequences that we can't edit - err_cnt += validate_no_non_editable_metadata(module_store, course_id, "sequential") + err_cnt += validate_no_non_editable_metadata( + module_store, course_id, "sequential" + ) # check for a presence of a course marketing video location_elements = course_id.split('/') - if Location(['i4x', location_elements[0], location_elements[1], 'about', 'video', None]) not in module_store.modules[course_id]: - print "WARN: Missing course marketing video. It is recommended that every course have a marketing video." + loc = Location([ + 'i4x', location_elements[0], location_elements[1], + 'about', 'video', None + ]) + if loc not in module_store.modules[course_id]: + print( + "WARN: Missing course marketing video. It is recommended " + "that every course have a marketing video." + ) warn_cnt += 1 - print "\n\n------------------------------------------\nVALIDATION SUMMARY: {0} Errors {1} Warnings\n".format(err_cnt, warn_cnt) + print("\n") + print("------------------------------------------") + print("VALIDATION SUMMARY: {err} Errors {warn} Warnings".format( + err=err_cnt, warn=warn_cnt) + ) if err_cnt > 0: - print "This course is not suitable for importing. Please fix courseware according to specifications before importing." + 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" + 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." + print("This course can be imported successfully.") return err_cnt