ext: Wrap extension state in a ExtensionData tuple
This allows us to do more of the data loading that might fail safely in the mopidy.ext module instead of having things spread all over the place. Note that only minimal changes have been made to __main__ to make things work. Further refactoring should follow.
This commit is contained in:
Thomas Adamcik
parent
5937cdc3b2
commit
5550785146
@ -68,19 +68,20 @@ def main():
|
||||
|
||||
installed_extensions = ext.load_extensions()
|
||||
|
||||
for extension in installed_extensions:
|
||||
ext_cmd = extension.get_command()
|
||||
if ext_cmd:
|
||||
ext_cmd.set(extension=extension)
|
||||
root_cmd.add_child(extension.ext_name, ext_cmd)
|
||||
for data in installed_extensions:
|
||||
if data.command:
|
||||
data.command.set(extension=data.command)
|
||||
root_cmd.add_child(data.extension.ext_name, data.command)
|
||||
|
||||
args = root_cmd.parse(mopidy_args)
|
||||
|
||||
create_file_structures_and_config(args, installed_extensions)
|
||||
check_old_locations()
|
||||
|
||||
# TODO: make config.load use extension data? or just pass in schema+def
|
||||
config, config_errors = config_lib.load(
|
||||
args.config_files, installed_extensions, args.config_overrides)
|
||||
args.config_files, [d.extension for d in installed_extensions],
|
||||
args.config_overrides)
|
||||
|
||||
verbosity_level = args.base_verbosity_level
|
||||
if args.verbosity_level:
|
||||
@ -90,8 +91,11 @@ def main():
|
||||
|
||||
extensions = {
|
||||
'validate': [], 'config': [], 'disabled': [], 'enabled': []}
|
||||
for extension in installed_extensions:
|
||||
if not ext.validate_extension(extension):
|
||||
for data in installed_extensions:
|
||||
extension = data.extension
|
||||
|
||||
# TODO: factor out all of this to a helper that can be tested
|
||||
if not ext.validate_extension(data.extension, data.entry_point):
|
||||
config[extension.ext_name] = {'enabled': False}
|
||||
config_errors[extension.ext_name] = {
|
||||
'enabled': 'extension disabled by self check.'}
|
||||
@ -109,6 +113,9 @@ def main():
|
||||
else:
|
||||
extensions['enabled'].append(extension)
|
||||
|
||||
# TODO: convert rest of code to use new ExtensionData
|
||||
installed_extensions = [d.extension for d in installed_extensions]
|
||||
|
||||
log_extension_info(installed_extensions, extensions['enabled'])
|
||||
|
||||
# Config and deps commands are simply special cased for now.
|
||||
|
||||
@ -11,6 +11,12 @@ from mopidy import config as config_lib, exceptions
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
_extension_data_fields = ['extension', 'entry_point', 'config_schema',
|
||||
'config_defaults', 'command']
|
||||
|
||||
ExtensionData = collections.namedtuple('ExtensionData', _extension_data_fields)
|
||||
|
||||
|
||||
class Extension(object):
|
||||
|
||||
"""Base class for Mopidy extensions"""
|
||||
@ -149,6 +155,8 @@ def load_extensions():
|
||||
logger.debug('Loading entry point: %s', entry_point)
|
||||
extension_class = entry_point.load(require=False)
|
||||
|
||||
# TODO: start using _extension_error_handling(...) pattern
|
||||
|
||||
try:
|
||||
if not issubclass(extension_class, Extension):
|
||||
continue # TODO: log this
|
||||
@ -160,19 +168,26 @@ def load_extensions():
|
||||
except Exception:
|
||||
continue # TODO: log this
|
||||
|
||||
extension.entry_point = entry_point
|
||||
# TODO: handle exceptions and validate result...
|
||||
config_schema = extension.get_config_schema()
|
||||
default_config = extension.get_default_config()
|
||||
command = extension.get_command()
|
||||
|
||||
installed_extensions.append(ExtensionData(
|
||||
extension, entry_point, config_schema, default_config, command))
|
||||
|
||||
# TODO: call validate_extension here?
|
||||
# TODO: do basic config tests like schema contains enabled?
|
||||
|
||||
# TODO: store: (instance, entry_point, command, schema, defaults)
|
||||
installed_extensions.append(extension)
|
||||
logger.debug(
|
||||
'Loaded extension: %s %s', extension.dist_name, extension.version)
|
||||
|
||||
names = (e.ext_name for e in installed_extensions)
|
||||
names = (ed.extension.ext_name for ed in installed_extensions)
|
||||
logger.debug('Discovered extensions: %s', ', '.join(names))
|
||||
return installed_extensions
|
||||
|
||||
|
||||
def validate_extension(extension):
|
||||
def validate_extension(extension, entry_point):
|
||||
"""Verify extension's dependencies and environment.
|
||||
|
||||
:param extensions: an extension to check
|
||||
@ -181,15 +196,15 @@ def validate_extension(extension):
|
||||
|
||||
logger.debug('Validating extension: %s', extension.ext_name)
|
||||
|
||||
if extension.ext_name != extension.entry_point.name:
|
||||
if extension.ext_name != entry_point.name:
|
||||
logger.warning(
|
||||
'Disabled extension %(ep)s: entry point name (%(ep)s) '
|
||||
'does not match extension name (%(ext)s)',
|
||||
{'ep': extension.entry_point.name, 'ext': extension.ext_name})
|
||||
{'ep': entry_point.name, 'ext': extension.ext_name})
|
||||
return False
|
||||
|
||||
try:
|
||||
extension.entry_point.require()
|
||||
entry_point.require()
|
||||
except pkg_resources.DistributionNotFound as ex:
|
||||
logger.info(
|
||||
'Disabled extension %s: Dependency %s not found',
|
||||
@ -202,7 +217,8 @@ def validate_extension(extension):
|
||||
'Disabled extension %s: %s required, but found %s at %s',
|
||||
extension.ext_name, required, found, found.location)
|
||||
else:
|
||||
logger.info('Disabled extension %s: %s', extension.ext_name, ex)
|
||||
logger.info(
|
||||
'Disabled extension %s: %s', extension.ext_name, ex)
|
||||
return False
|
||||
|
||||
try:
|
||||
|
||||
@ -8,12 +8,20 @@ import pytest
|
||||
|
||||
from mopidy import config, exceptions, ext
|
||||
|
||||
from tests import IsA, any_unicode
|
||||
|
||||
|
||||
class TestExtension(ext.Extension):
|
||||
dist_name = 'Mopidy-Foobar'
|
||||
ext_name = 'foobar'
|
||||
version = '1.2.3'
|
||||
|
||||
def get_default_config(self):
|
||||
return '[foobar]\nenabled = true'
|
||||
|
||||
|
||||
any_testextension = IsA(TestExtension)
|
||||
|
||||
|
||||
# ext.Extension
|
||||
|
||||
@ -69,9 +77,11 @@ def test_load_extensions(mock_entry_points):
|
||||
|
||||
mock_entry_points.return_value = [mock_entry_point]
|
||||
|
||||
extensions = ext.load_extensions()
|
||||
assert len(extensions) == 1
|
||||
assert isinstance(extensions[0], TestExtension)
|
||||
expected = ext.ExtensionData(
|
||||
any_testextension, mock_entry_point, IsA(config.ConfigSchema),
|
||||
any_unicode, None)
|
||||
|
||||
assert ext.load_extensions() == [expected]
|
||||
|
||||
|
||||
@mock.patch('pkg_resources.iter_entry_points')
|
||||
@ -110,87 +120,62 @@ def test_load_extensions_creating_instance_fails(mock_entry_points):
|
||||
assert [] == ext.load_extensions()
|
||||
|
||||
|
||||
@mock.patch('pkg_resources.iter_entry_points')
|
||||
def test_load_extensions_store_entry_point(mock_entry_points):
|
||||
mock_entry_point = mock.Mock()
|
||||
mock_entry_point.load.return_value = TestExtension
|
||||
mock_entry_points.return_value = [mock_entry_point]
|
||||
|
||||
extensions = ext.load_extensions()
|
||||
assert len(extensions) == 1
|
||||
assert extensions[0].entry_point == mock_entry_point
|
||||
|
||||
|
||||
# ext.validate_extension
|
||||
|
||||
def test_validate_extension_name_mismatch():
|
||||
ep = mock.Mock()
|
||||
ep.name = 'barfoo'
|
||||
|
||||
@pytest.fixture
|
||||
def ext_data():
|
||||
extension = TestExtension()
|
||||
extension.entry_point = ep
|
||||
|
||||
assert not ext.validate_extension(extension)
|
||||
entry_point = mock.Mock()
|
||||
entry_point.name = extension.ext_name
|
||||
|
||||
schema = extension.get_config_schema()
|
||||
defaults = extension.get_default_config()
|
||||
command = extension.get_command()
|
||||
|
||||
return ext.ExtensionData(extension, entry_point, schema, defaults, command)
|
||||
|
||||
|
||||
def test_validate_extension_distribution_not_found():
|
||||
ep = mock.Mock()
|
||||
ep.name = 'foobar'
|
||||
ep.require.side_effect = pkg_resources.DistributionNotFound
|
||||
|
||||
extension = TestExtension()
|
||||
extension.entry_point = ep
|
||||
|
||||
assert not ext.validate_extension(extension)
|
||||
def test_validate_extension_name_mismatch(ext_data):
|
||||
ext_data.entry_point.name = 'barfoo'
|
||||
assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
|
||||
|
||||
|
||||
def test_validate_extension_version_conflict():
|
||||
ep = mock.Mock()
|
||||
ep.name = 'foobar'
|
||||
ep.require.side_effect = pkg_resources.VersionConflict
|
||||
|
||||
extension = TestExtension()
|
||||
extension.entry_point = ep
|
||||
|
||||
assert not ext.validate_extension(extension)
|
||||
def test_validate_extension_distribution_not_found(ext_data):
|
||||
error = pkg_resources.DistributionNotFound
|
||||
ext_data.entry_point.require.side_effect = error
|
||||
assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
|
||||
|
||||
|
||||
def test_validate_extension_exception():
|
||||
ep = mock.Mock()
|
||||
ep.name = 'foobar'
|
||||
ep.require.side_effect = Exception
|
||||
def test_validate_extension_version_conflict(ext_data):
|
||||
ext_data.entry_point.require.side_effect = pkg_resources.VersionConflict
|
||||
assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
|
||||
|
||||
extension = TestExtension()
|
||||
extension.entry_point = ep
|
||||
|
||||
def test_validate_extension_exception(ext_data):
|
||||
ext_data.entry_point.require.side_effect = Exception
|
||||
|
||||
# We trust that entry points are well behaved, so exception will bubble.
|
||||
with pytest.raises(Exception):
|
||||
assert not ext.validate_extension(extension)
|
||||
assert not ext.validate_extension(
|
||||
ext_data.extension, ext_data.entry_point)
|
||||
|
||||
|
||||
def test_validate_extension_instance_validate_env_ext_error():
|
||||
ep = mock.Mock()
|
||||
ep.name = 'foobar'
|
||||
|
||||
extension = TestExtension()
|
||||
extension.entry_point = ep
|
||||
|
||||
def test_validate_extension_instance_validate_env_ext_error(ext_data):
|
||||
extension = ext_data.extension
|
||||
with mock.patch.object(extension, 'validate_environment') as validate:
|
||||
validate.side_effect = exceptions.ExtensionError('error')
|
||||
|
||||
assert not ext.validate_extension(extension)
|
||||
assert not ext.validate_extension(
|
||||
ext_data.extension, ext_data.entry_point)
|
||||
validate.assert_called_once_with()
|
||||
|
||||
|
||||
def test_validate_extension_instance_validate_env_exception():
|
||||
ep = mock.Mock()
|
||||
ep.name = 'foobar'
|
||||
|
||||
extension = TestExtension()
|
||||
extension.entry_point = ep
|
||||
|
||||
def test_validate_extension_instance_validate_env_exception(ext_data):
|
||||
extension = ext_data.extension
|
||||
with mock.patch.object(extension, 'validate_environment') as validate:
|
||||
validate.side_effect = Exception
|
||||
|
||||
assert not ext.validate_extension(extension)
|
||||
assert not ext.validate_extension(
|
||||
ext_data.extension, ext_data.entry_point)
|
||||
validate.assert_called_once_with()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user