From 7a26ec6b928075896f87f59559241bede1f38701 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Mon, 7 Jun 2021 17:52:05 -0400 Subject: [PATCH] refactor: move xml_importer-specific exceptions into xml_importer This is a minor refactoring in order to remove a code dependency of ./xmodule on ./cms --- cms/djangoapps/contentstore/errors.py | 2 -- cms/djangoapps/contentstore/exceptions.py | 30 ---------------- cms/djangoapps/contentstore/tasks.py | 3 +- .../views/tests/test_import_export.py | 9 +++-- .../xmodule/modulestore/xml_importer.py | 35 ++++++++++++++++++- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/cms/djangoapps/contentstore/errors.py b/cms/djangoapps/contentstore/errors.py index 54f425d27d..b1ad51f873 100644 --- a/cms/djangoapps/contentstore/errors.py +++ b/cms/djangoapps/contentstore/errors.py @@ -5,8 +5,6 @@ from django.utils.translation import ugettext as _ COURSE_ALREADY_EXIST = _('Aborting import because a course with this id: {} already exists.') COURSE_PERMISSION_DENIED = _('Permission denied. You do not have write access to this course.') -ERROR_WHILE_READING = _('Error while reading {}. Check file for XML errors.') -FAILED_TO_IMPORT_MODULE = _('Failed to import module: {} at location: {}') FILE_MISSING = _('Could not find the {0} file in the package.') FILE_NOT_FOUND = _('Uploaded Tar file not found. Try again.') INVALID_FILE_TYPE = _('We only support uploading a .tar.gz file.') diff --git a/cms/djangoapps/contentstore/exceptions.py b/cms/djangoapps/contentstore/exceptions.py index 380a2a6373..bf33bad8f9 100644 --- a/cms/djangoapps/contentstore/exceptions.py +++ b/cms/djangoapps/contentstore/exceptions.py @@ -1,45 +1,15 @@ """ A common module for managing exceptions. Helps to avoid circular references """ -from .errors import ERROR_WHILE_READING, FAILED_TO_IMPORT_MODULE - - -class CourseImportException(Exception): - """Base exception class for course import workflows.""" - - def __init__(self): - super().__init__(self.description) # pylint: disable=no-member - - -class ErrorReadingFileException(CourseImportException): - """ - Raised when error occurs while trying to read a file. - """ - - def __init__(self, filename, **kwargs): - self.description = ERROR_WHILE_READING.format(filename) - super().__init__(**kwargs) - - -class ModuleFailedToImport(CourseImportException): - """ - Raised when a module is failed to import. - """ - - def __init__(self, display_name, location, **kwargs): - self.description = FAILED_TO_IMPORT_MODULE.format(display_name, location) - super().__init__(**kwargs) class AssetNotFoundException(Exception): """ Raised when asset not found """ - pass # lint-amnesty, pylint: disable=unnecessary-pass class AssetSizeTooLargeException(Exception): """ Raised when the size of an uploaded asset exceeds the maximum size limit. """ - pass # lint-amnesty, pylint: disable=unnecessary-pass diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 82ae009702..9186ad7fd4 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -61,9 +61,8 @@ from xmodule.modulestore import COURSE_ROOT, LIBRARY_ROOT from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import DuplicateCourseError, InvalidProctoringProvider, ItemNotFoundError from xmodule.modulestore.xml_exporter import export_course_to_xml, export_library_to_xml -from xmodule.modulestore.xml_importer import import_course_from_xml, import_library_from_xml +from xmodule.modulestore.xml_importer import CourseImportException, import_course_from_xml, import_library_from_xml -from .exceptions import CourseImportException from .outlines import update_outline_from_modulestore from .outlines_regenerate import CourseOutlineRegenerate from .toggles import bypass_olx_failure_enabled diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index ed1df9665c..38e0388b7c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -28,7 +28,6 @@ from storages.backends.s3boto3 import S3Boto3Storage from user_tasks.models import UserTaskStatus from cms.djangoapps.contentstore import errors as import_error -from cms.djangoapps.contentstore.exceptions import ErrorReadingFileException, ModuleFailedToImport from cms.djangoapps.contentstore.storage import course_import_export_storage from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from cms.djangoapps.contentstore.tests.utils import CourseTestCase @@ -45,7 +44,13 @@ from xmodule.modulestore.exceptions import DuplicateCourseError, InvalidProctori from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from xmodule.modulestore.tests.utils import SPLIT_MODULESTORE_SETUP, TEST_DATA_DIR, MongoContentstoreBuilder from xmodule.modulestore.xml_exporter import export_course_to_xml, export_library_to_xml -from xmodule.modulestore.xml_importer import CourseImportManager, import_course_from_xml, import_library_from_xml +from xmodule.modulestore.xml_importer import ( + CourseImportManager, + ErrorReadingFileException, + import_course_from_xml, + import_library_from_xml, + ModuleFailedToImport, +) TASK_LOGGER = 'cms.djangoapps.contentstore.tasks.LOGGER' TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index ba3c80275d..07b184cf6c 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -29,6 +29,7 @@ import re from abc import abstractmethod import xblock +from django.utils.translation import ugettext as _ from lxml import etree from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryLocator @@ -37,7 +38,6 @@ from xblock.core import XBlockMixin from xblock.fields import Reference, ReferenceList, ReferenceValueDict, Scope from xblock.runtime import DictKeyValueStore, KvsFieldData -from cms.djangoapps.contentstore.exceptions import ErrorReadingFileException, ModuleFailedToImport from common.djangoapps.util.monitoring import monitor_import_failure from xmodule.assetstore import AssetMetadata from xmodule.contentstore.content import StaticContent @@ -61,6 +61,39 @@ log = logging.getLogger(__name__) DEFAULT_STATIC_CONTENT_SUBDIR = 'static' +class CourseImportException(Exception): + """ + Base exception class for course import workflows. + """ + + def __init__(self): + super().__init__(self.description) # pylint: disable=no-member + + +class ErrorReadingFileException(CourseImportException): + """ + Raised when error occurs while trying to read a file. + """ + + MESSAGE_TEMPLATE = _('Error while reading {}. Check file for XML errors.') + + def __init__(self, filename, **kwargs): + self.description = self.MESSAGE_TEMPLATE.format(filename) + super().__init__(**kwargs) + + +class ModuleFailedToImport(CourseImportException): + """ + Raised when a module is failed to import. + """ + + MESSAGE_TEMPLATE = _('Failed to import module: {} at location: {}') + + def __init__(self, display_name, location, **kwargs): + self.description = self.MESSAGE_TEMPLATE.format(display_name, location) + super().__init__(**kwargs) + + class LocationMixin(XBlockMixin): """ Adds a `location` property to an :class:`XBlock` so it is more compatible