From c54d8a81bf56eb5f2ad7f63ccf45477cce737c05 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Thu, 9 Jun 2022 02:06:59 +1000 Subject: [PATCH] refactor: deprecate node_path attribute of ModuleSystem (#30447) The node_path attribute & constructor argument of the ModuleSystem is deprecated without any replacement service or fallback as there doesn't seem to be any core blocks using it. It also removes the references to node_path from the LMS settings, the LoncapaModuleSystem and the XBlock runtime shim. Co-authored-by: Agrendalath --- common/lib/capa/capa/capa_problem.py | 2 -- common/lib/capa/capa/tests/helpers.py | 1 - common/lib/xmodule/xmodule/capa_module.py | 3 --- common/lib/xmodule/xmodule/tests/__init__.py | 1 - common/lib/xmodule/xmodule/x_module.py | 15 +++++++++++++-- lms/djangoapps/courseware/module_render.py | 1 - lms/envs/common.py | 10 ---------- openedx/core/djangoapps/xblock/runtime/shims.py | 10 ---------- tox.ini | 2 -- 9 files changed, 13 insertions(+), 32 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index cc1ea427f5..30295b0919 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -104,7 +104,6 @@ class LoncapaSystem(object): get_python_lib_zip, DEBUG, i18n, - node_path, render_template, resources_fs, seed, # Why do we do this if we have self.seed? @@ -119,7 +118,6 @@ class LoncapaSystem(object): self.get_python_lib_zip = get_python_lib_zip self.DEBUG = DEBUG # pylint: disable=invalid-name 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? diff --git a/common/lib/capa/capa/tests/helpers.py b/common/lib/capa/capa/tests/helpers.py index 1779168c1a..0d30b1b60f 100644 --- a/common/lib/capa/capa/tests/helpers.py +++ b/common/lib/capa/capa/tests/helpers.py @@ -73,7 +73,6 @@ def test_capa_system(render_template=None): get_python_lib_zip=lambda: None, DEBUG=True, 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, diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 064c0c81f8..250fd0974f 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -618,7 +618,6 @@ class ProblemBlock( get_python_lib_zip=None, DEBUG=None, i18n=self.runtime.service(self, "i18n"), - node_path=None, render_template=None, resources_fs=self.runtime.resources_fs, seed=None, @@ -680,7 +679,6 @@ class ProblemBlock( get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, self.runtime.course_id)), DEBUG=None, i18n=self.runtime.service(self, "i18n"), - node_path=None, render_template=None, resources_fs=self.runtime.resources_fs, seed=1, @@ -829,7 +827,6 @@ class ProblemBlock( get_python_lib_zip=sandbox_service.get_python_lib_zip, DEBUG=self.runtime.DEBUG, 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? diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index fd38c6f64d..206784dc24 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -168,7 +168,6 @@ def get_test_system( ), 'replace_urls': replace_url_service }, - node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), course_id=course_id, error_descriptor_class=ErrorBlock, descriptor_runtime=descriptor_system, diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 307d7c8155..ffc6a58ff9 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1609,6 +1609,18 @@ class ModuleSystemShim: ) return self.resources_fs + @property + def node_path(self): + """ + Path to node_modules. Doesn't seem to be used by any ModuleSystem dependent core XBlock anymore. + + Deprecated. + """ + warnings.warn( + 'node_path is deprecated. Please use other methods of finding the node_modules location.', + DeprecationWarning, stacklevel=3 + ) + class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ @@ -1626,7 +1638,7 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, def __init__( self, static_url, track_function, get_module, descriptor_runtime, debug=False, hostname="", publish=None, - node_path="", course_id=None, error_descriptor_class=None, + course_id=None, error_descriptor_class=None, field_data=None, rebind_noauth_module_to_user=None, **kwargs): """ @@ -1668,7 +1680,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, self.get_module = get_module self.DEBUG = self.debug = debug self.HOSTNAME = self.hostname = hostname - self.node_path = node_path self.course_id = course_id if publish: diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 62157ec648..4d7ad48c79 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -731,7 +731,6 @@ def get_module_system_for_user( user=user, debug=settings.DEBUG, hostname=settings.SITE_NAME, - node_path=settings.NODE_PATH, publish=publish, course_id=course_id, # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) diff --git a/lms/envs/common.py b/lms/envs/common.py index f268ad6b2a..fe21b28aad 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1087,16 +1087,6 @@ NODE_MODULES_ROOT = REPO_ROOT / "node_modules" DATA_DIR = COURSES_ROOT -# For Node.js - -system_node_path = os.environ.get("NODE_PATH", NODE_MODULES_ROOT) - -node_paths = [ - COMMON_ROOT / "static/js/vendor", - system_node_path, -] -NODE_PATH = ':'.join(node_paths) - # For geolocation ip database GEOIP_PATH = REPO_ROOT / "common/static/data/geoip/GeoLite2-Country.mmdb" # Where to look for a status message diff --git a/openedx/core/djangoapps/xblock/runtime/shims.py b/openedx/core/djangoapps/xblock/runtime/shims.py index 41b028de42..637ce63cda 100644 --- a/openedx/core/djangoapps/xblock/runtime/shims.py +++ b/openedx/core/djangoapps/xblock/runtime/shims.py @@ -145,16 +145,6 @@ class RuntimeShim: ) return self.resources_fs - @property - def node_path(self): - """ - Get the path to Node.js - - Seems only to be used by capa. Remove this if capa can be refactored. - """ - # TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here. - return getattr(settings, 'NODE_PATH', None) # Only defined in the LMS - def render_template(self, template_name, dictionary, namespace='main'): """ Render a mako template diff --git a/tox.ini b/tox.ini index 63cbf6c4ef..0bcc2ae0bb 100644 --- a/tox.ini +++ b/tox.ini @@ -39,8 +39,6 @@ passenv = LMS_CFG REVISION_CFG MOZ_HEADLESS - NODE_PATH - NODE_VIRTUAL_ENV NO_PREREQ_INSTALL NO_PYTHON_UNINSTALL NPM_CONFIG_PREFIX