Skip to content

Commit

Permalink
Remove YAML config for plugins (closes #145)
Browse files Browse the repository at this point in the history
  • Loading branch information
johnmaguire committed Sep 1, 2019
1 parent f7e9406 commit bfc0e42
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 71 deletions.
4 changes: 0 additions & 4 deletions cardinal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ class ConfigNotFoundError(CardinalException):
"""Raised when an expected plugin config isn't found."""


class AmbiguousConfigError(CardinalException):
"""Raised when multiple configs exist for a plugin."""


class EventAlreadyExistsError(CardinalException):
"""Raised durring attempt to register an event name already registered."""

Expand Down
Empty file.

This file was deleted.

6 changes: 0 additions & 6 deletions cardinal/fixtures/fake_plugins/config_invalid_yaml/plugin.py

This file was deleted.

52 changes: 6 additions & 46 deletions cardinal/plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
import linecache
import random
import json
import yaml
from collections import defaultdict

from cardinal.exceptions import (
AmbiguousConfigError,
CommandNotFoundError,
ConfigNotFoundError,
EventAlreadyExistsError,
Expand Down Expand Up @@ -287,19 +285,17 @@ def _close_plugin_instance(self, plugin):
raise PluginError("Unknown arguments for close function")

def _load_plugin_config(self, plugin):
"""Loads a JSON or YAML config for a given plugin
"""Loads a JSON config for a given plugin
Keyword arguments:
plugin -- Name of plugin to load config for.
Raises:
AmbiguousConfigError -- Raised when two configs exist for plugin.
ConfigNotFoundError -- Raised when expected config isn't found.
"""
# Initialize variable to hold plugin config
json_config = None
yaml_config = None
config = None

# Attempt to load and parse JSON config file
file_ = os.path.join(
Expand All @@ -309,7 +305,7 @@ def _load_plugin_config(self, plugin):
)
try:
f = open(file_, 'r')
json_config = json.load(f)
config = json.load(f)
f.close()
# File did not exist or we can't open it for another reason
except IOError:
Expand All @@ -322,44 +318,14 @@ def _load_plugin_config(self, plugin):
"Invalid JSON in %s, skipping it" % file_
)

# Attempt to load and parse YAML config file
file_ = os.path.join(
self.plugins_directory,
plugin,
'config.yaml'
)
try:
f = open(file_, 'r')
yaml_config = yaml.load(f, Loader=yaml.FullLoader)
f.close()
except IOError:
self.logger.debug(
"Can't open %s - maybe it doesn't exist?" % file_
)
except yaml.YAMLError:
self.logger.warning(
"Invalid YAML in %s, skipping it" % file_
)
# Loaded YAML successfully
else:
# If we already loaded JSON, this is a problem because we won't
# know which config to use.
if json_config is not None:
raise AmbiguousConfigError(
"Found both a JSON and YAML config for plugin"
)

# No JSON config, found YAML config, return it
return yaml_config

# If neither config was found, raise an exception
if not yaml_config and not json_config:
if not config:
raise ConfigNotFoundError(
"No config found for plugin: %s" % plugin
)

# Return JSON config, since YAML config wasn't found
return json_config
# Return config
return config

def _get_plugin_commands(self, instance):
"""Find the commands in a plugin and return them as callables.
Expand Down Expand Up @@ -496,12 +462,6 @@ def load(self, plugins):
config = None
try:
config = self._load_plugin_config(plugin)
except AmbiguousConfigError:
# If two configs exist for the plugin, bail on loading
self.logger.exception("Could not load plugin: %s" % plugin)
failed_plugins.append(plugin)

continue
except ConfigNotFoundError:
self.logger.debug(
"No config found for plugin: %s" % plugin
Expand Down
3 changes: 0 additions & 3 deletions cardinal/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ def test_exceptions():
with pytest.raises(exceptions.CardinalException):
raise exceptions.ConfigNotFoundError

with pytest.raises(exceptions.CardinalException):
raise exceptions.AmbiguousConfigError

with pytest.raises(exceptions.CardinalException):
raise exceptions.EventAlreadyExistsError

Expand Down
17 changes: 6 additions & 11 deletions cardinal/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,16 @@ def test_load_plugins_not_a_list_or_string_typeerror(self, plugins):
'setup_missing',
# This plugin's setup() function takes three arguments
'setup_too_many_arguments',
# This plugin has both a config.yaml and config.json
'config_ambiguous',
])
def test_load_invalid(self, plugins):
self.assert_load_failed(plugins)

@pytest.mark.parametrize("plugins", [
'valid',
['valid'], # test list format
# This plugin has both a config.yaml and config.json which used to be
# disallowed, but now config.yaml is ignored
'config_ambiguous',
])
def test_load_valid(self, plugins):
self.assert_load_success(plugins)
Expand All @@ -156,13 +157,6 @@ def test_load_invalid_json_config(self):
# invalid json should be ignored
assert self.plugin_manager.plugins[name]['config'] is None

def test_load_invalid_yaml_config(self):
name = 'config_invalid_yaml'
self.assert_load_success(name) # no error for some reason

# invalid json should be ignored
assert self.plugin_manager.plugins[name]['config'] is None

def test_get_config_unloaded_plugin(self):
name = 'nonexistent_plugin'
with pytest.raises(exceptions.ConfigNotFoundError):
Expand All @@ -181,11 +175,12 @@ def test_get_config_json(self):

self.plugin_manager.get_config(name) == {'test': True}

def test_get_config_yaml(self):
def test_get_config_yaml_ignored(self):
name = 'config_valid_yaml'
self.assert_load_success(name, assert_config_is_none=False)

self.plugin_manager.get_config(name) == {'test': True}
with pytest.raises(exceptions.ConfigNotFoundError):
self.plugin_manager.get_config(name)

def test_plugin_iteration(self):
plugins = [
Expand Down

0 comments on commit bfc0e42

Please sign in to comment.