diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 42a4535adb..8b05d8786f 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -198,7 +198,6 @@ def _preview_module_system(request, descriptor, field_data): static_url=settings.STATIC_URL, # TODO (cpennington): Do we want to track how instructors are using the preview problems? track_function=lambda event_type, event: None, - filestore=descriptor.runtime.resources_fs, get_module=partial(_load_preview_module, request), debug=True, mixins=settings.XBLOCK_MIXINS, diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 9172c50062..cc1ea427f5 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -103,10 +103,10 @@ class LoncapaSystem(object): can_execute_unsafe_code, get_python_lib_zip, DEBUG, - filestore, i18n, node_path, render_template, + resources_fs, seed, # Why do we do this if we have self.seed? STATIC_URL, xqueue, @@ -118,10 +118,10 @@ class LoncapaSystem(object): self.can_execute_unsafe_code = can_execute_unsafe_code self.get_python_lib_zip = get_python_lib_zip self.DEBUG = DEBUG # pylint: disable=invalid-name - self.filestore = filestore self.i18n = i18n self.node_path = node_path self.render_template = render_template + self.resources_fs = resources_fs self.seed = seed # Why do we do this if we have self.seed? self.STATIC_URL = STATIC_URL # pylint: disable=invalid-name self.xqueue = xqueue @@ -814,8 +814,8 @@ class LoncapaProblem(object): filename = inc.get('file') if six.PY3 else inc.get('file').decode('utf-8') if filename is not None: try: - # open using LoncapaSystem OSFS filestore - ifp = self.capa_system.filestore.open(filename) + # open using LoncapaSystem OSFS filesystem + ifp = self.capa_system.resources_fs.open(filename) except Exception as err: # lint-amnesty, pylint: disable=broad-except log.warning( 'Error %s in problem xml include: %s', @@ -823,7 +823,7 @@ class LoncapaProblem(object): etree.tostring(inc, pretty_print=True) ) log.warning( - 'Cannot find file %s in %s', filename, self.capa_system.filestore + 'Cannot find file %s in %s', filename, self.capa_system.resources_fs ) # if debugging, don't fail - just log error # TODO (vshnayder): need real error handling, display to users @@ -876,9 +876,9 @@ class LoncapaProblem(object): continue # path is an absolute path or a path relative to the data dir - dir = os.path.join(self.capa_system.filestore.root_path, dir) - # Check that we are within the filestore tree. - reldir = os.path.relpath(dir, self.capa_system.filestore.root_path) + dir = os.path.join(self.capa_system.resources_fs.root_path, dir) + # Check that we are within the resources_fs tree. + reldir = os.path.relpath(dir, self.capa_system.resources_fs.root_path) if ".." in reldir: log.warning("Ignoring Python directory outside of course: %r", dir) continue diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 3e4d6fc937..6f71427642 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -3268,7 +3268,7 @@ class SchematicResponse(LoncapaResponse): answer_src = answer.get('src') if answer_src is not None: # Untested; never used - self.code = self.capa_system.filestore.open('src/' + answer_src).read() + self.code = self.capa_system.resources_fs.open('src/' + answer_src).read() else: self.code = answer.text diff --git a/common/lib/capa/capa/tests/helpers.py b/common/lib/capa/capa/tests/helpers.py index c923d0bdd9..1779168c1a 100644 --- a/common/lib/capa/capa/tests/helpers.py +++ b/common/lib/capa/capa/tests/helpers.py @@ -72,10 +72,10 @@ def test_capa_system(render_template=None): can_execute_unsafe_code=lambda: False, get_python_lib_zip=lambda: None, DEBUG=True, - filestore=fs.osfs.OSFS(os.path.join(TEST_DIR, "test_files")), i18n=gettext.NullTranslations(), node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), render_template=render_template or tst_render_template, + resources_fs=fs.osfs.OSFS(os.path.join(TEST_DIR, "test_files")), seed=0, STATIC_URL='/dummy-static/', STATUS_CLASS=Status, diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index bbb8195a49..dee9ac29c1 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -300,7 +300,7 @@ class CapaHtmlRenderTest(unittest.TestCase): self.assertRegex(the_html, r"
\s*
") def _create_test_file(self, path, content_str): # lint-amnesty, pylint: disable=missing-function-docstring - test_fp = self.capa_system.filestore.open(path, "w") + test_fp = self.capa_system.resources_fs.open(path, "w") test_fp.write(content_str) test_fp.close() diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index efff7e9011..b23dbf5527 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -616,10 +616,10 @@ class ProblemBlock( can_execute_unsafe_code=None, get_python_lib_zip=None, DEBUG=None, - filestore=self.runtime.resources_fs, i18n=self.runtime.service(self, "i18n"), node_path=None, render_template=None, + resources_fs=self.runtime.resources_fs, seed=None, STATIC_URL=None, xqueue=None, @@ -678,10 +678,10 @@ class ProblemBlock( can_execute_unsafe_code=lambda: None, get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, self.runtime.course_id)), DEBUG=None, - filestore=self.runtime.resources_fs, i18n=self.runtime.service(self, "i18n"), node_path=None, render_template=None, + resources_fs=self.runtime.resources_fs, seed=1, STATIC_URL=None, xqueue=None, @@ -827,10 +827,10 @@ class ProblemBlock( can_execute_unsafe_code=sandbox_service.can_execute_unsafe_code, get_python_lib_zip=sandbox_service.get_python_lib_zip, DEBUG=self.runtime.DEBUG, - filestore=self.runtime.filestore, i18n=self.runtime.service(self, "i18n"), node_path=self.runtime.node_path, render_template=self.runtime.service(self, 'mako').render_template, + resources_fs=self.runtime.resources_fs, seed=seed, # Why do we do this if we have self.seed? STATIC_URL=self.runtime.STATIC_URL, xqueue=self.runtime.service(self, 'xqueue'), diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 90fe9a4e58..a2df7f1a3d 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -46,7 +46,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li module_data: a dict mapping Location -> json that was cached from the underlying modulestore """ - # needed by capa_problem (as runtime.filestore via this.resources_fs) + # needed by capa_problem (as runtime.resources_fs via this.resources_fs) if course_entry.course_key.course: root = modulestore.fs_root / course_entry.course_key.org / course_entry.course_key.course / course_entry.course_key.run # lint-amnesty, pylint: disable=line-too-long else: diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index bb70153158..02d4c8f980 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -82,6 +82,10 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method def get_asides(self, block): return [] + @property + def resources_fs(self): + return Mock(name='TestModuleSystem.resources_fs', root_path='.'), + def __repr__(self): """ Custom hacky repr. @@ -149,7 +153,6 @@ def get_test_system( static_url='/static', track_function=Mock(name='get_test_system.track_function'), get_module=get_module, - filestore=Mock(name='get_test_system.filestore', root_path='.'), debug=True, hostname="edx.org", services={ diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index fb59a44f03..7592cf1572 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -2004,6 +2004,19 @@ class ModuleSystemShim: if replace_urls_service: return partial(replace_urls_service.replace_urls) + @property + def filestore(self): + """ + A filestore ojbect. Defaults to an instance of OSFS based at settings.DATA_DIR. + + Deprecated in favor of runtime.resources_fs property. + """ + warnings.warn( + 'runtime.filestore is deprecated. Please use the runtime.resources_fs service instead.', + DeprecationWarning, stacklevel=3, + ) + return self.resources_fs + class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ @@ -2020,9 +2033,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, def __init__( self, static_url, track_function, get_module, - descriptor_runtime, filestore=None, - debug=False, hostname="", publish=None, node_path="", - course_id=None, error_descriptor_class=None, + descriptor_runtime, debug=False, hostname="", publish=None, + node_path="", course_id=None, error_descriptor_class=None, field_data=None, rebind_noauth_module_to_user=None, **kwargs): """ @@ -2039,9 +2051,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, module instance object. If the current user does not have access to that location, returns None. - filestore - A filestore ojbect. Defaults to an instance of OSFS based - at settings.DATA_DIR. - descriptor_runtime - A `DescriptorSystem` to use for loading xblocks by id course_id - the course_id containing this module @@ -2064,7 +2073,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, self.STATIC_URL = static_url self.track_function = track_function - self.filestore = filestore self.get_module = get_module self.DEBUG = self.debug = debug self.HOSTNAME = self.hostname = hostname diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index f9ee3c8d21..62157ec648 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -727,8 +727,6 @@ def get_module_system_for_user( system = LmsModuleSystem( track_function=track_function, static_url=settings.STATIC_URL, - # TODO (cpennington): Figure out how to share info between systems - filestore=descriptor.runtime.resources_fs, get_module=inner_get_module, user=user, debug=settings.DEBUG, diff --git a/openedx/core/djangoapps/xblock/runtime/shims.py b/openedx/core/djangoapps/xblock/runtime/shims.py index 38425a8923..41b028de42 100644 --- a/openedx/core/djangoapps/xblock/runtime/shims.py +++ b/openedx/core/djangoapps/xblock/runtime/shims.py @@ -137,9 +137,12 @@ class RuntimeShim: @property def filestore(self): """ - Alternate name for 'resources_fs'. Not sure if either name is deprecated - but we should deprecate one :) + Alternate name for 'resources_fs'. """ + warnings.warn( + 'filestore is deprecated. Please use runtime.resources_fs instead.', + DeprecationWarning, stacklevel=3, + ) return self.resources_fs @property @@ -223,7 +226,7 @@ class RuntimeShim: # XBlock repo at xblock.reference.plugins.FSService and is available in # the old runtime as the 'fs' service. warnings.warn( - "Use of legacy runtime.resources_fs or .filestore won't be able to find resources.", + "Use of legacy runtime.resources_fs won't be able to find resources.", stacklevel=3, ) fake_fs = MemoryFS() diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index e9b7a47ca7..fafa203a4e 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -294,9 +294,9 @@ def add_staff_markup(user, disable_staff_debug_info, block, view, frag, context) histogram = None render_histogram = False - if settings.FEATURES.get('ENABLE_LMS_MIGRATION') and hasattr(block.runtime, 'filestore'): + if settings.FEATURES.get('ENABLE_LMS_MIGRATION') and hasattr(block.runtime, 'resources_fs'): [filepath, filename] = getattr(block, 'xml_attributes', {}).get('filename', ['', None]) - osfs = block.runtime.filestore + osfs = block.runtime.resources_fs if filename is not None and osfs.exists(filename): # if original, unmangled filename exists then use it (github # doesn't like symlinks)