From a8a8ae3286db2a76d36c7aa6743e1d5f96729c9f Mon Sep 17 00:00:00 2001 From: Irtaza Akram <51848298+irtazaakram@users.noreply.github.com> Date: Thu, 13 Feb 2025 17:43:07 +0500 Subject: [PATCH] fix: replace pkg_resources with importlib.resources (#36213) --- cms/djangoapps/contentstore/tasks.py | 4 ++-- cms/pytest.ini | 4 ---- common/djangoapps/edxmako/paths.py | 8 ++++--- common/test/pytest.ini | 4 ---- openedx/core/lib/logsettings.py | 10 -------- openedx/core/lib/xblock_pipeline/finder.py | 23 ++++++++++--------- .../test_external_xblocks.py | 4 ++-- scripts/xblock/list-installed.py | 9 ++++---- setup.cfg | 4 ---- xmodule/modulestore/django.py | 8 +++---- .../modulestore/tests/test_django_utils.py | 7 +++--- xmodule/x_module.py | 21 +++++++++-------- 12 files changed, 45 insertions(+), 61 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 88f3ad77f1..68e0c78279 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -10,11 +10,11 @@ import re import shutil import tarfile from datetime import datetime, timezone +from importlib.metadata import entry_points from tempfile import NamedTemporaryFile, mkdtemp import aiohttp import olxcleaner -import pkg_resources from ccx_keys.locator import CCXLocator from celery import shared_task from celery.utils.log import get_task_logger @@ -95,7 +95,7 @@ LOGGER = get_task_logger(__name__) FILE_READ_CHUNK = 1024 # bytes FULL_COURSE_REINDEX_THRESHOLD = 1 ALL_ALLOWED_XBLOCKS = frozenset( - [entry_point.name for entry_point in pkg_resources.iter_entry_points("xblock.v1")] + [entry_point.name for entry_point in entry_points(group="xblock.v1")] ) diff --git a/cms/pytest.ini b/cms/pytest.ini index c18bf1d66f..ee9263d63c 100644 --- a/cms/pytest.ini +++ b/cms/pytest.ini @@ -16,10 +16,6 @@ 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 8a1ab6ea75..3da9844eed 100644 --- a/common/djangoapps/edxmako/paths.py +++ b/common/djangoapps/edxmako/paths.py @@ -4,8 +4,8 @@ Set up lookup paths for mako templates. import contextlib import hashlib import os +import importlib.resources as resources -import pkg_resources from django.conf import settings from mako.exceptions import TopLevelLookupException from mako.lookup import TemplateLookup @@ -122,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, `pkg_resources` is used to look up the directory + If `package` is specified, `importlib.resources` is used to look up the directory inside the given package. Otherwise `directory` is assumed to be a path in the filesystem. """ @@ -136,7 +136,9 @@ def add_lookup(namespace, directory, package=None, prepend=False): encoding_errors='replace', ) if package: - directory = pkg_resources.resource_filename(package, directory) + with resources.as_file(resources.files(package.rsplit('.', 1)[0]) / directory) as dir_path: + directory = str(dir_path) + templates.add_directory(directory, prepend=prepend) diff --git a/common/test/pytest.ini b/common/test/pytest.ini index 5df48c9325..d93a742172 100644 --- a/common/test/pytest.ini +++ b/common/test/pytest.ini @@ -15,10 +15,6 @@ 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/openedx/core/lib/logsettings.py b/openedx/core/lib/logsettings.py index f813be5c8f..ee78b26623 100644 --- a/openedx/core/lib/logsettings.py +++ b/openedx/core/lib/logsettings.py @@ -144,16 +144,6 @@ 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 da5f4e5330..4893e5e048 100644 --- a/openedx/core/lib/xblock_pipeline/finder.py +++ b/openedx/core/lib/xblock_pipeline/finder.py @@ -4,13 +4,13 @@ Django pipeline finder for handling static assets required by XBlocks. import os from datetime import datetime +import importlib.resources as resources from django.contrib.staticfiles import utils from django.contrib.staticfiles.finders import BaseFinder from django.contrib.staticfiles.storage import FileSystemStorage from django.core.files.storage import Storage from django.utils import timezone -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,8 @@ class XBlockPackageStorage(Storage): """ Returns a file system filename for the specified file name. """ - return resource_filename(self.module, os.path.join(self.base_dir, name)) + with resources.as_file(resources.files(self.module.rsplit('.', 1)[0]) / self.base_dir / name) as file_path: + return str(file_path) def exists(self, path): # lint-amnesty, pylint: disable=arguments-differ """ @@ -46,8 +47,7 @@ class XBlockPackageStorage(Storage): """ if self.base_dir is None: return False - - return resource_exists(self.module, os.path.join(self.base_dir, path)) + return (resources.files(self.module.rsplit('.', 1)[0]) / self.base_dir / path).exists() def listdir(self, path): """ @@ -55,13 +55,14 @@ class XBlockPackageStorage(Storage): """ directories = [] 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 resource_isdir(self.module, os.path.join(self.base_dir, path, item)): - directories.append(item) - else: - files.append(item) + base_path = resources.files(self.module.rsplit('.', 1)[0]) / self.base_dir / path + if base_path.is_dir(): + for item in base_path.iterdir(): + if item.suffix not in [".py", ".pyc", ".scss"]: + if item.is_dir(): + directories.append(item.name) + else: + files.append(item.name) 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 c4fc4b74e3..01bdf2133c 100644 --- a/openedx/tests/xblock_integration/test_external_xblocks.py +++ b/openedx/tests/xblock_integration/test_external_xblocks.py @@ -9,7 +9,7 @@ That be the dragon here. """ -import pkg_resources +from importlib.metadata import entry_points class DuplicateXBlockTest(Exception): @@ -37,7 +37,7 @@ class InvalidTestName(Exception): xblock_loaded = False # pylint: disable=invalid-name -for entrypoint in pkg_resources.iter_entry_points(group="xblock.test.v0"): +for entrypoint in 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 c1152d749d..fb19bf6db9 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 """ -import pkg_resources +from importlib.metadata import entry_points def get_without_builtins(): @@ -12,11 +12,10 @@ def get_without_builtins(): """ xblocks = [ entry_point.name - for entry_point in pkg_resources.iter_entry_points('xblock.v1') - if not entry_point.module_name.startswith('xmodule') + for entry_point in entry_points(group='xblock.v1') + if not entry_point.value.startswith('xmodule') ] - xblocks = sorted(xblocks) - return xblocks + return sorted(xblocks) def main(): diff --git a/setup.cfg b/setup.cfg index 987f447453..bf789997c0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,10 +17,6 @@ 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 f36c0c35a9..c15f03a4e3 100644 --- a/xmodule/modulestore/django.py +++ b/xmodule/modulestore/django.py @@ -6,10 +6,10 @@ Passes settings.MODULESTORE as kwargs to MongoModuleStore from contextlib import contextmanager from importlib import import_module +import importlib.resources as resources import gettext import logging -from pkg_resources import resource_filename import re # lint-amnesty, pylint: disable=wrong-import-order from django.conf import settings @@ -422,9 +422,9 @@ class XBlockI18nService: return 'django', xblock_locale_path # Pre-OEP-58 translations within the XBlock pip packages are deprecated but supported. - 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 + with resources.as_file(resources.files(xblock_module_name) / 'translations') as deprecated_xblock_locale_path: + # The `text` domain was used for XBlocks pre-OEP-58. + return 'text', str(deprecated_xblock_locale_path) def get_javascript_i18n_catalog_url(self, block): """ diff --git a/xmodule/modulestore/tests/test_django_utils.py b/xmodule/modulestore/tests/test_django_utils.py index 86c3a08df5..f8fba427b4 100644 --- a/xmodule/modulestore/tests/test_django_utils.py +++ b/xmodule/modulestore/tests/test_django_utils.py @@ -2,6 +2,7 @@ Tests for the modulestore.django module """ +from pathlib import Path from unittest.mock import patch import django.utils.translation @@ -23,19 +24,19 @@ 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.resource_filename', return_value='/lib/my_legacy_xblock/translations') +@patch('importlib.resources.files', return_value=Path('/lib/my_legacy_xblock')) 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 `pkg_resources.resource_filename` function. + accessible via the `importlib.resources.files` function. """ i18n_service = XBlockI18nService() block = mock_modern_xblock['legacy_xblock'] domain, path = i18n_service.get_python_locale(block) - assert path == '/lib/my_legacy_xblock/translations', 'Backward compatible with pe-OEP-58.' + assert path == '/lib/my_legacy_xblock/translations', 'Backward compatible with pre-OEP-58.' assert domain == 'text', 'Use the legacy `text` domain for backward compatibility with old XBlocks.' diff --git a/xmodule/x_module.py b/xmodule/x_module.py index d25012275a..d1d23342b7 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1,5 +1,6 @@ # lint-amnesty, pylint: disable=missing-module-docstring +import importlib.resources as resources import logging import os import time @@ -13,7 +14,6 @@ from django.conf import settings from lxml import etree from opaque_keys.edx.asides import AsideDefinitionKeyV2, AsideUsageKeyV2 from opaque_keys.edx.keys import UsageKey -from pkg_resources import resource_isdir, resource_filename from web_fragments.fragment import Fragment from webob import Response from webob.multidict import MultiDict @@ -856,16 +856,16 @@ class ResourceTemplates: 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) # lint-amnesty, pylint: disable=no-member - if not resource_isdir(__name__, dirname): + template_path = resources.files(__name__.rsplit('.', 1)[0]) / dirname + + if not template_path.is_dir(): log.warning("No resource directory {dir} found when loading {cls_name} templates".format( dir=dirname, cls_name=cls.__name__, )) - return None - else: - return dirname - else: - return None + return + return dirname + return @classmethod def get_template_dirpaths(cls): @@ -874,8 +874,11 @@ class ResourceTemplates: """ template_dirpaths = [] template_dirname = cls.get_template_dir() - if template_dirname and resource_isdir(__name__, template_dirname): - template_dirpaths.append(resource_filename(__name__, template_dirname)) + if template_dirname: + template_path = resources.files(__name__.rsplit('.', 1)[0]) / template_dirname + if template_path.is_dir(): + with resources.as_file(template_path) as template_real_path: + template_dirpaths.append(str(template_real_path)) custom_template_dir = cls.get_custom_template_dir() if custom_template_dir: