Skip to content

Commit

Permalink
Switch to using importlib from pkg_resources (#1469)
Browse files Browse the repository at this point in the history
* 馃毧 Clean: Correct test name typo
* 馃摝 Improve: Use importlib, not pkg_resources, for entry_points plugin discovery
* 馃摝 Improve: Use importlib, not pkg_resources, in sample and test plugins
* 馃摝 Improve: Use importlib, not inspect, for builtin and contrib adapter discovery
* 馃摝 Improve: Log "could not load plugin" exception
* 馃摝 Improve: Remove test setUp pkg_resources dependency
* Fix: Tests use posix to avoid Windows path slashes
* 馃毧 Clean: Only install importlib backport for python<=3.7
* 馃摝 Improve: Float importlib_metadata to avoid resolve conflicts

Signed-off-by: Mike Mahony <mikemahony@pixar.com>
  • Loading branch information
mikemahony committed Nov 10, 2022
1 parent b6fbca2 commit 5e27e78
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 94 deletions.
8 changes: 4 additions & 4 deletions docs/tutorials/otio-env-variables.md
Expand Up @@ -18,10 +18,10 @@ OTIO_DEFAULT_MEDIA_LINKER
The name of the default media linker to use after reading a file, if `""` then no
media linker is automatically invoked.
OTIO_DISABLE_PKG_RESOURCE_PLUGINS
By default, OTIO will use the `pkg_resource` entry_points mechanism to discover plugins
that have been installed into the current python environment. `pkg_resources`, however, can
be slow in certain cases, so for users who wish to disable this behavior, this variable can be set to 1.
OTIO_DISABLE_ENTRYPOINTS_PLUGINS
By default, OTIO will use the `importlib.metadata` entry_points mechanism to discover plugins
that have been installed into the current python environment. For users who wish to disable this
behavior, this variable can be set to 1.
OTIO_DEFAULT_TARGET_VERSION_FAMILY_LABEL
If no downgrade arguments are passed to `write_to_file`/`write_to_string`, use the downgrade manifest
Expand Down
6 changes: 4 additions & 2 deletions examples/sample_plugin/otio_counter/__init__.py
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright Contributors to the OpenTimelineIO project
import pkg_resources

import importlib.resources

from opentimelineio.plugins import manifest

Expand Down Expand Up @@ -62,6 +63,7 @@ def plugin_manifest():
# XXX: note, this doesn't get called. For an example of this working,
# see the mockplugin unit test.

filepath = importlib.resources.files(__package__) / "plugin_manifest.json"
return manifest.manifest_from_string(
pkg_resources.resource_string(__name__, 'plugin_manifest.json')
filepath.read_text()
)
1 change: 1 addition & 0 deletions setup.py
Expand Up @@ -361,6 +361,7 @@ def run(self):

install_requires=[
'pyaaf2>=1.4,<1.7',
'importlib_metadata>=1.4; python_version < "3.8"',
],
entry_points={
'console_scripts': [
Expand Down
119 changes: 62 additions & 57 deletions src/py-opentimelineio/opentimelineio/plugins/manifest.py
Expand Up @@ -3,21 +3,17 @@

"""OTIO Python Plugin Manifest system: locates plugins to OTIO."""

from importlib import resources
import inspect
import logging
import os
from pathlib import Path

# In some circumstances pkg_resources has bad performance characteristics.
# Using the envirionment variable: $OTIO_DISABLE_PKG_RESOURCE_PLUGINS disables
# OpenTimelineIO's import and of use of the pkg_resources module.
if os.environ.get("OTIO_DISABLE_PKG_RESOURCE_PLUGINS", False):
pkg_resources = None
else:
try:
# on some python interpreters, pkg_resources is not available
import pkg_resources
except ImportError:
pkg_resources = None
try:
from importlib import metadata
except ImportError:
# For python 3.7
import importlib_metadata as metadata

from .. import (
core,
Expand Down Expand Up @@ -250,11 +246,14 @@ def load_manifest():

result.extend(manifest_from_file(json_path))

# setuptools.pkg_resources based plugins
if pkg_resources:
for plugin in pkg_resources.iter_entry_points(
"opentimelineio.plugins"
):
if not os.environ.get("OTIO_DISABLE_ENTRYPOINTS_PLUGINS"):
try:
entry_points = metadata.entry_points(group='opentimelineio.plugins')
except TypeError:
# For python <= 3.9
entry_points = metadata.entry_points().get('opentimelineio.plugins', [])

for plugin in entry_points:
plugin_name = plugin.name
try:
plugin_entry_point = plugin.load()
Expand All @@ -276,72 +275,78 @@ def load_manifest():
plugin_manifest._update_plugin_source(manifest_path)

except AttributeError:
if not pkg_resources.resource_exists(
plugin.module_name,
'plugin_manifest.json'
):
raise

filepath = os.path.abspath(
pkg_resources.resource_filename(
plugin.module_name,
'plugin_manifest.json'
)
)
name = plugin_entry_point.__name__

if filepath in result.source_files:
try:
filepath = resources.files(name) / "plugin_manifest.json"
except AttributeError:
# For python <= 3.7
with resources.path(name, "plugin_manifest.json") as p:
filepath = Path(p)

if filepath.as_posix() in result.source_files:
continue

manifest_stream = pkg_resources.resource_stream(
plugin.module_name,
'plugin_manifest.json'
)
plugin_manifest = core.deserialize_json_from_string(
manifest_stream.read().decode('utf-8')
filepath.read_text()
)
manifest_stream.close()
plugin_manifest._update_plugin_source(filepath.as_posix())
plugin_manifest.source_files.append(filepath.as_posix())

plugin_manifest._update_plugin_source(filepath)
plugin_manifest.source_files.append(filepath)

except Exception:
except Exception as e:
logging.exception(
f"could not load plugin: {plugin_name}"
f"could not load plugin: {plugin_name}. Exception is: {e}"
)
continue

result.extend(plugin_manifest)
else:
# XXX: Should we print some kind of warning that pkg_resources isn't
# available?
pass
logging.debug(
"OTIO_DISABLE_ENTRYPOINTS_PLUGINS is set. "
"Entry points plugings have been disabled."
)

# the builtin plugin manifest
builtin_manifest_path = os.path.join(
os.path.dirname(os.path.dirname(inspect.getsourcefile(core))),
"adapters",
"builtin_adapters.plugin_manifest.json"
)
try:
builtin_manifest_path = (
resources.files("opentimelineio.adapters")
/ "builtin_adapters.plugin_manifest.json"
).as_posix()
except AttributeError:
# For python <= 3.7
with resources.path(
"opentimelineio.adapters",
"builtin_adapters.plugin_manifest.json"
) as p:
builtin_manifest_path = p.as_posix()

if os.path.abspath(builtin_manifest_path) not in result.source_files:
plugin_manifest = manifest_from_file(builtin_manifest_path)
result.extend(plugin_manifest)

# the contrib plugin manifest (located in the opentimelineio_contrib package)
try:
import opentimelineio_contrib as otio_c
try:
contrib_manifest_path = (
resources.files("opentimelineio_contrib.adapters")
/ "contrib_adapters.plugin_manifest.json"
).as_posix()
except AttributeError:
# For python <= 3.7
with resources.path(
"opentimelineio_contrib.adapters",
"contrib_adapters.plugin_manifest.json"
) as p:
contrib_manifest_path = p.as_posix()

except ModuleNotFoundError:
logging.debug("no opentimelineio_contrib.adapters package found")

contrib_manifest_path = os.path.join(
os.path.dirname(inspect.getsourcefile(otio_c)),
"adapters",
"contrib_adapters.plugin_manifest.json"
)
else:
if os.path.abspath(contrib_manifest_path) not in result.source_files:
contrib_manifest = manifest_from_file(contrib_manifest_path)
result.extend(contrib_manifest)

except ImportError:
pass

# force the schemadefs to load and add to schemadef module namespace
for s in result.schemadefs:
s.module()
Expand Down
15 changes: 10 additions & 5 deletions tests/baselines/plugin_module/otio_mockplugin/__init__.py
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright Contributors to the OpenTimelineIO project

import pkg_resources
from importlib import resources
from pathlib import Path

from opentimelineio.plugins import manifest

Expand All @@ -20,9 +21,13 @@


def plugin_manifest():
try:
filepath = resources.files(__package__) / "unusually_named_plugin_manifest.json"
except AttributeError:
# For python <= 3.7
with resources.path(__package__, "unusually_named_plugin_manifest.json") as p:
filepath = Path(p)

return manifest.manifest_from_string(
pkg_resources.resource_string(
__name__,
'unusually_named_plugin_manifest.json'
)
filepath.read_text()
)
87 changes: 61 additions & 26 deletions tests/test_plugin_detection.py
Expand Up @@ -5,13 +5,19 @@

import unittest
import os
import pkg_resources
from pathlib import Path
import sys

from unittest import mock

from importlib import reload as import_reload

try:
import importlib.metadata as metadata
except ImportError:
# For python 3.7
import importlib_metadata as metadata

import opentimelineio as otio
from tests import baseline_reader

Expand All @@ -23,41 +29,33 @@ def setUp(self):
os.path.normpath(baseline_reader.path_to_baseline_directory()),
'plugin_module',
)
self.mock_module_manifest_path = os.path.join(
self.mock_module_manifest_path = Path(
mock_module_path,
"otio_jsonplugin",
"plugin_manifest.json"
)
).absolute().as_posix()

self.override_adapter_manifest_path = os.path.join(
self.override_adapter_manifest_path = Path(
mock_module_path,
"otio_override_adapter",
"plugin_manifest.json"
)
).absolute().as_posix()

# Create a WorkingSet as if the module were installed
entries = [mock_module_path] + pkg_resources.working_set.entries
entries = [mock_module_path] + sys.path

self.original_sysmodule_keys = set(sys.modules.keys())

self.sys_patch = mock.patch('sys.path', entries)
self.sys_patch.start()

working_set = pkg_resources.WorkingSet(entries)

# linker from the entry point
self.entry_patcher = mock.patch(
'pkg_resources.iter_entry_points',
working_set.iter_entry_points
)
self.entry_patcher.start()

def tearDown(self):
self.sys_patch.stop()
self.entry_patcher.stop()
if 'otio_mockplugin' in sys.modules:
del sys.modules['otio_mockplugin']

if 'otio_override_adapter' in sys.modules:
del sys.modules['otio_override_adapter']
# Remove any modules added under test. We cannot replace sys.modules with
# a copy from setUp. For more, see: https://bugs.python.org/msg188914
for key in set(sys.modules.keys()) ^ self.original_sysmodule_keys:
sys.modules.pop(key)

def test_detect_plugin(self):
"""This manifest uses the plugin_manifest function"""
Expand All @@ -80,7 +78,8 @@ def test_detect_plugin(self):
for linker in man.media_linkers:
self.assertIsInstance(linker, otio.media_linker.MediaLinker)

def test_overrride_adapter(self):
def test_override_adapter(self):

# Test that entrypoint plugins load before builtin and contrib
man = otio.plugins.manifest.load_manifest()

Expand All @@ -93,7 +92,7 @@ def test_overrride_adapter(self):

# Override adapter should be the first adapter found
manifest = adapters[0].plugin_info_map().get('from manifest', None)
self.assertEqual(manifest, os.path.abspath(self.override_adapter_manifest_path))
self.assertEqual(manifest, self.override_adapter_manifest_path)

self.assertTrue(
any(
Expand All @@ -102,8 +101,8 @@ def test_overrride_adapter(self):
)
)

def test_pkg_resources_disabled(self):
os.environ["OTIO_DISABLE_PKG_RESOURCE_PLUGINS"] = "1"
def test_entrypoints_disabled(self):
os.environ["OTIO_DISABLE_ENTRYPOINTS_PLUGINS"] = "1"
import_reload(otio.plugins.manifest)

# detection of the environment variable happens on import, force a
Expand All @@ -113,11 +112,11 @@ def test_pkg_resources_disabled(self):

# override adapter should not be loaded either
with self.assertRaises(AssertionError):
self.test_overrride_adapter()
self.test_override_adapter()

# remove the environment variable and reload again for usage in the
# other tests
del os.environ["OTIO_DISABLE_PKG_RESOURCE_PLUGINS"]
del os.environ["OTIO_DISABLE_ENTRYPOINTS_PLUGINS"]
import_reload(otio.plugins.manifest)

def test_detect_plugin_json_manifest(self):
Expand Down Expand Up @@ -185,6 +184,42 @@ def test_deduplicate_env_variable_paths(self):
else:
del os.environ['OTIO_PLUGIN_MANIFEST_PATH']

def test_plugin_load_failure(self):
"""When a plugin fails to load, ensure the exception message
is logged (and no exception thrown)
"""

sys.modules['otio_mock_bad_module'] = mock.Mock(
name='otio_mock_bad_module',
plugin_manifest=mock.Mock(
side_effect=Exception("Mock Exception")
)
)

entry_points = mock.patch(
'opentimelineio.plugins.manifest.metadata.entry_points',
return_value=[
metadata.EntryPoint(
'mock_bad_module',
'otio_mock_bad_module',
'opentimelineio.plugins'
)
]
)

with self.assertLogs() as cm, entry_points:
# Load the above mock entrypoint, expect it to fail and log
otio.plugins.manifest.load_manifest()

load_errors = [
r for r in cm.records
if r.message.startswith(
"could not load plugin: mock_bad_module. "
"Exception is: Mock Exception"
)
]
self.assertEqual(len(load_errors), 1)


if __name__ == '__main__':
unittest.main()

0 comments on commit 5e27e78

Please sign in to comment.