From 88b95e1f0ede59bd5c824ba7fb757f521f121929 Mon Sep 17 00:00:00 2001 From: Jesper Hodge <19345795+jesperhodge@users.noreply.github.com> Date: Mon, 3 Feb 2025 13:31:26 -0500 Subject: [PATCH] Revert "Replace pkg_resources with importlib.resources" --- cms/djangoapps/contentstore/tasks.py | 4 +-- cms/pytest.ini | 4 +++ common/djangoapps/edxmako/paths.py | 12 +++------ common/test/pytest.ini | 4 +++ docs/decisions/0000-static-asset-plan.rst | 2 +- .../0019-oep-58-atlas-translations-design.rst | 6 ++--- openedx/core/lib/logsettings.py | 10 +++++++ openedx/core/lib/xblock_pipeline/finder.py | 16 ++++++------ .../test_external_xblocks.py | 4 +-- scripts/xblock/list-installed.py | 9 ++++--- setup.cfg | 4 +++ xmodule/modulestore/django.py | 5 ++-- .../modulestore/tests/test_django_utils.py | 4 +-- xmodule/x_module.py | 26 ++++++++----------- 14 files changed, 62 insertions(+), 48 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 032747f6368e..bb220c371711 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -11,7 +11,7 @@ from tempfile import NamedTemporaryFile, mkdtemp import olxcleaner -from importlib.metadata import entry_points +import pkg_resources from ccx_keys.locator import CCXLocator from celery import shared_task from celery.utils.log import get_task_logger @@ -85,7 +85,7 @@ FILE_READ_CHUNK = 1024 # bytes FULL_COURSE_REINDEX_THRESHOLD = 1 ALL_ALLOWED_XBLOCKS = frozenset( - [entry_point.name for entry_point in entry_points(group="xblock.v1")] + [entry_point.name for entry_point in pkg_resources.iter_entry_points("xblock.v1")] ) diff --git a/cms/pytest.ini b/cms/pytest.ini index ee9263d63c27..c18bf1d66f31 100644 --- a/cms/pytest.ini +++ b/cms/pytest.ini @@ -16,6 +16,10 @@ filterwarnings = ignore:.*You can remove default_app_config.*:PendingDeprecationWarning # ABC deprecation Warning comes from libsass ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated.*:DeprecationWarning:sass + # declare_namespace Warning comes from XBlock https://github.com/openedx/XBlock/issues/641 + # and also due to dependency: https://github.com/PyFilesystem/pyfilesystem2 + ignore:Deprecated call to `pkg_resources.declare_namespace.*:DeprecationWarning + ignore:.*pkg_resources is deprecated as an API.*:DeprecationWarning ignore:'etree' is deprecated. Use 'xml.etree.ElementTree' instead.:DeprecationWarning:wiki norecursedirs = envs diff --git a/common/djangoapps/edxmako/paths.py b/common/djangoapps/edxmako/paths.py index 3779e6b67cf4..8a1ab6ea75de 100644 --- a/common/djangoapps/edxmako/paths.py +++ b/common/djangoapps/edxmako/paths.py @@ -5,8 +5,7 @@ import hashlib import os -from importlib.resources import files -from pathlib import Path +import pkg_resources from django.conf import settings from mako.exceptions import TopLevelLookupException from mako.lookup import TemplateLookup @@ -123,7 +122,7 @@ def add_lookup(namespace, directory, package=None, prepend=False): """ Adds a new mako template lookup directory to the given namespace. - If `package` is specified, `importlib.resources` is used to look up the directory + If `package` is specified, `pkg_resources` is used to look up the directory inside the given package. Otherwise `directory` is assumed to be a path in the filesystem. """ @@ -137,11 +136,8 @@ def add_lookup(namespace, directory, package=None, prepend=False): encoding_errors='replace', ) if package: - package, module_path = package.split('.', 1) - module_dir = str(Path(module_path).parent) - directory = files(package).joinpath(module_dir, directory) - - templates.add_directory(str(directory), prepend=prepend) + directory = pkg_resources.resource_filename(package, directory) + templates.add_directory(directory, prepend=prepend) @request_cached() diff --git a/common/test/pytest.ini b/common/test/pytest.ini index d93a7421722b..5df48c9325ee 100644 --- a/common/test/pytest.ini +++ b/common/test/pytest.ini @@ -15,6 +15,10 @@ filterwarnings = ignore:.*You can remove default_app_config.*:PendingDeprecationWarning # ABC deprecation Warning comes from libsass ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated.*:DeprecationWarning:sass + # declare_namespace Warning comes from XBlock https://github.com/openedx/XBlock/issues/641 + # and also due to dependency: https://github.com/PyFilesystem/pyfilesystem2 + ignore:Deprecated call to `pkg_resources.declare_namespace.*:DeprecationWarning + ignore:.*pkg_resources is deprecated as an API.*:DeprecationWarning ignore:'etree' is deprecated. Use 'xml.etree.ElementTree' instead.:DeprecationWarning:wiki norecursedirs = .cache diff --git a/docs/decisions/0000-static-asset-plan.rst b/docs/decisions/0000-static-asset-plan.rst index eb64df858f60..00fcc0726ad7 100644 --- a/docs/decisions/0000-static-asset-plan.rst +++ b/docs/decisions/0000-static-asset-plan.rst @@ -84,7 +84,7 @@ mechanism: ``openedx.core.lib.xblock_pipeline.finder.XBlockPipelineFinder`` Custom finder that accesses and extracts assets from pip-installed XBlocks via - ``importlib.resources``. + ``pkg_resources``. ``openedx.core.storage.DevelopmentStorage/ProductionStorage`` Custom ``FileStorage`` classes that mostly exist for theme-awareness. diff --git a/docs/decisions/0019-oep-58-atlas-translations-design.rst b/docs/decisions/0019-oep-58-atlas-translations-design.rst index 3c36fd5d1979..a59e774a0535 100644 --- a/docs/decisions/0019-oep-58-atlas-translations-design.rst +++ b/docs/decisions/0019-oep-58-atlas-translations-design.rst @@ -305,7 +305,7 @@ be updated to support the new ``XBlockI18nService`` def _get_statici18n_js_url(): """ Returns the Javascript translation file for the currently selected language, if any found by - `importlib.resources.files` + `pkg_resources` """ lang_code = translation.get_language() if not lang_code: @@ -320,8 +320,8 @@ be updated to support the new ``XBlockI18nService`` text_js = 'public/js/translations/{lang_code}/text.js' country_code = lang_code.split('-')[0] for code in (translation.to_locale(lang_code), lang_code, country_code): - if files(loader.module_name).joinpath(text_js.format(lang_code=code)).is_file(): - return text_js.format(lang_code=code) + if pkg_resources.resource_exists(loader.module_name, text_js.format(lang_code=code)): + return text_js.format(lang_code=code) return None diff --git a/openedx/core/lib/logsettings.py b/openedx/core/lib/logsettings.py index ee78b26623ad..f813be5c8fb7 100644 --- a/openedx/core/lib/logsettings.py +++ b/openedx/core/lib/logsettings.py @@ -144,6 +144,16 @@ def log_python_warnings(): category=DeprecationWarning, module="sass", ) + warnings.filterwarnings( + 'ignore', + 'Deprecated call to `pkg_resources.declare_namespace.*', + category=DeprecationWarning, + ) + warnings.filterwarnings( + 'ignore', + '.*pkg_resources is deprecated as an API.*', + category=DeprecationWarning, + ) warnings.filterwarnings( 'ignore', "'etree' is deprecated. Use 'xml.etree.ElementTree' instead.", category=DeprecationWarning, module='wiki' diff --git a/openedx/core/lib/xblock_pipeline/finder.py b/openedx/core/lib/xblock_pipeline/finder.py index 63c7122a6c31..da5f4e53303b 100644 --- a/openedx/core/lib/xblock_pipeline/finder.py +++ b/openedx/core/lib/xblock_pipeline/finder.py @@ -10,7 +10,7 @@ from django.contrib.staticfiles.storage import FileSystemStorage from django.core.files.storage import Storage from django.utils import timezone -from importlib.resources import files +from pkg_resources import resource_exists, resource_filename, resource_isdir, resource_listdir from xblock.core import XBlock from openedx.core.lib.xblock_utils import xblock_resource_pkg @@ -38,7 +38,7 @@ def path(self, name): """ Returns a file system filename for the specified file name. """ - return str(files(self.module).joinpath(self.base_dir, name)) + return resource_filename(self.module, os.path.join(self.base_dir, name)) def exists(self, path): # lint-amnesty, pylint: disable=arguments-differ """ @@ -47,22 +47,22 @@ def exists(self, path): # lint-amnesty, pylint: disable=arguments-differ if self.base_dir is None: return False - return os.path.exists(os.path.join(self.base_dir, path)) + return resource_exists(self.module, os.path.join(self.base_dir, path)) def listdir(self, path): """ Lists the directories beneath the specified path. """ directories = [] - files_p = [] - for item in files(self.module).joinpath(self.base_dir, path).iterdir(): + files = [] + for item in resource_listdir(self.module, os.path.join(self.base_dir, path)): __, file_extension = os.path.splitext(item) if file_extension not in [".py", ".pyc", ".scss"]: - if files(self.module).joinpath(self.base_dir, path, item).is_dir(): + if resource_isdir(self.module, os.path.join(self.base_dir, path, item)): directories.append(item) else: - files_p.append(item) - return directories, files_p + files.append(item) + return directories, files def open(self, name, mode='rb'): """ diff --git a/openedx/tests/xblock_integration/test_external_xblocks.py b/openedx/tests/xblock_integration/test_external_xblocks.py index 01bdf2133cce..c4fc4b74e397 100644 --- a/openedx/tests/xblock_integration/test_external_xblocks.py +++ b/openedx/tests/xblock_integration/test_external_xblocks.py @@ -9,7 +9,7 @@ """ -from importlib.metadata import entry_points +import pkg_resources class DuplicateXBlockTest(Exception): @@ -37,7 +37,7 @@ class InvalidTestName(Exception): xblock_loaded = False # pylint: disable=invalid-name -for entrypoint in entry_points(group="xblock.test.v0"): +for entrypoint in pkg_resources.iter_entry_points(group="xblock.test.v0"): plugin = entrypoint.load() classname = plugin.__name__ if classname in globals(): diff --git a/scripts/xblock/list-installed.py b/scripts/xblock/list-installed.py index 9537ac6c49b4..c1152d749d58 100644 --- a/scripts/xblock/list-installed.py +++ b/scripts/xblock/list-installed.py @@ -1,7 +1,7 @@ """ Lookup list of installed XBlocks, to aid XBlock developers """ -from importlib.metadata import entry_points +import pkg_resources def get_without_builtins(): @@ -12,10 +12,11 @@ def get_without_builtins(): """ xblocks = [ entry_point.name - for entry_point in entry_points(group='xblock.v1') - if not entry_point.module.startswith('xmodule') + for entry_point in pkg_resources.iter_entry_points('xblock.v1') + if not entry_point.module_name.startswith('xmodule') ] - return sorted(xblocks) + xblocks = sorted(xblocks) + return xblocks def main(): diff --git a/setup.cfg b/setup.cfg index bf789997c030..987f4474531f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,6 +17,10 @@ filterwarnings = ignore:Instead access HTTPResponse.headers directly.*:DeprecationWarning:elasticsearch # ABC deprecation Warning comes from libsass ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated.*:DeprecationWarning:sass + # declare_namespace Warning comes from XBlock https://github.com/openedx/XBlock/issues/641 + # and also due to dependency: https://github.com/PyFilesystem/pyfilesystem2 + ignore:Deprecated call to `pkg_resources.declare_namespace.*:DeprecationWarning + ignore:.*pkg_resources is deprecated as an API.*:DeprecationWarning ignore:'etree' is deprecated. Use 'xml.etree.ElementTree' instead.:DeprecationWarning:wiki junit_family = xunit2 diff --git a/xmodule/modulestore/django.py b/xmodule/modulestore/django.py index 270aa1043141..f36c0c35a90d 100644 --- a/xmodule/modulestore/django.py +++ b/xmodule/modulestore/django.py @@ -9,7 +9,7 @@ import gettext import logging -from importlib.resources import path as resources_path +from pkg_resources import resource_filename import re # lint-amnesty, pylint: disable=wrong-import-order from django.conf import settings @@ -422,8 +422,7 @@ def get_python_locale(self, block): return 'django', xblock_locale_path # Pre-OEP-58 translations within the XBlock pip packages are deprecated but supported. - deprecated_xblock_locale_path = str(resources_path(xblock_module_name, 'translations')) - + deprecated_xblock_locale_path = resource_filename(xblock_module_name, 'translations') # The `text` domain was used for XBlocks pre-OEP-58. return 'text', deprecated_xblock_locale_path diff --git a/xmodule/modulestore/tests/test_django_utils.py b/xmodule/modulestore/tests/test_django_utils.py index 9d0ed78ea332..86c3a08df503 100644 --- a/xmodule/modulestore/tests/test_django_utils.py +++ b/xmodule/modulestore/tests/test_django_utils.py @@ -23,13 +23,13 @@ def test_get_python_locale_with_atlas_oep58_translations(mock_modern_xblock): assert domain == 'django', 'Uses django domain when atlas locale is found.' -@patch('xmodule.modulestore.django.resources_path', return_value='/lib/my_legacy_xblock/translations') +@patch('xmodule.modulestore.django.resource_filename', return_value='/lib/my_legacy_xblock/translations') def test_get_python_locale_with_bundled_translations(mock_modern_xblock): """ Ensure that get_python_locale() falls back to XBlock internal translations if atlas translations weren't pulled. Pre-OEP-58 translations were stored in the `translations` directory of the XBlock which is - accessible via the `importlib.resources.path` function. + accessible via the `pkg_resources.resource_filename` function. """ i18n_service = XBlockI18nService() block = mock_modern_xblock['legacy_xblock'] diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 00fcc9203048..d25012275a5e 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -13,8 +13,7 @@ from lxml import etree from opaque_keys.edx.asides import AsideDefinitionKeyV2, AsideUsageKeyV2 from opaque_keys.edx.keys import UsageKey -from importlib.resources import files, as_file -from pathlib import Path as P +from pkg_resources import resource_isdir, resource_filename from web_fragments.fragment import Fragment from webob import Response from webob.multidict import MultiDict @@ -856,35 +855,32 @@ def templates(cls): @classmethod def get_template_dir(cls): # lint-amnesty, pylint: disable=missing-function-docstring if getattr(cls, 'template_dir_name', None): - dirname = os.path.join('templates', cls.template_dir_name) - if not os.path.isdir(os.path.join(os.path.dirname(__file__), dirname)): + dirname = os.path.join('templates', cls.template_dir_name) # lint-amnesty, pylint: disable=no-member + if not resource_isdir(__name__, dirname): log.warning("No resource directory {dir} found when loading {cls_name} templates".format( dir=dirname, cls_name=cls.__name__, )) return None - return dirname - return None + else: + return dirname + else: + return None @classmethod def get_template_dirpaths(cls): """ - Returns a list of directories containing resource templates. + Returns of list of directories containing resource templates. """ template_dirpaths = [] template_dirname = cls.get_template_dir() - package, module_path = __name__.split('.', 1) - module_dir = str(P(module_path).parent) - module_dir = "" if module_dir == "." else module_dir - file_dirs = files(package).joinpath(module_dir, template_dirname or "") - if template_dirname and file_dirs.is_dir(): - with as_file(file_dirs) as path: - template_dirpaths.append(path) + if template_dirname and resource_isdir(__name__, template_dirname): + template_dirpaths.append(resource_filename(__name__, template_dirname)) custom_template_dir = cls.get_custom_template_dir() if custom_template_dir: template_dirpaths.append(custom_template_dir) - return [str(td) for td in template_dirpaths] + return template_dirpaths @classmethod def get_custom_template_dir(cls):