jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/363746 )
Change subject: Move configuration loader from cli to main module
......................................................................
Move configuration loader from cli to main module
* add a cumin.Config class
* move the parse_config helper to cumin's main module from the cli one,
to allow to easily load the configuration also when it's used as a
library.
Bug: T169640
Change-Id: I924f0731268e15249be30e5b5482918f2d853265
---
M cumin/__init__.py
M cumin/cli.py
M cumin/tests/unit/test_cli.py
A cumin/tests/unit/test_init.py
4 files changed, 94 insertions(+), 52 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, but someone else must approve
jenkins-bot: Verified
Volans: Looks good to me, approved
diff --git a/cumin/__init__.py b/cumin/__init__.py
index 9e70e4c..941208c 100644
--- a/cumin/__init__.py
+++ b/cumin/__init__.py
@@ -1,6 +1,8 @@
"""Automation and orchestration framework written in Python."""
from pkg_resources import DistributionNotFound, get_distribution
+import yaml
+
try:
__version__ = get_distribution(__name__).version
@@ -10,3 +12,43 @@
class CuminError(Exception):
"""Base Exception class for all Cumin's custom Exceptions."""
+
+
+class Config(dict):
+ """Singleton-like dictionary class to load the configuration from a given
path only once."""
+
+ _instances = {} # Keep track of different loaded configurations
+
+ def __new__(cls, config='/etc/cumin/config.yaml'):
+ """Load the given configuration if not already loaded and return it.
+
+ Called by Python's data model for each new instantiation of the class.
+
+ Arguments:
+ config -- path to the configuration file to load. [optional, default:
/etc/cumin/config.yaml]
+ """
+ if config not in cls._instances:
+ cls._instances[config] = parse_config(config)
+
+ return cls._instances[config]
+
+
+def parse_config(config_file):
+ """Parse the YAML configuration file.
+
+ Arguments:
+ config_file -- the path of the configuration file to load
+ """
+ try:
+ with open(config_file, 'r') as f:
+ config = yaml.safe_load(f)
+ except IOError as e:
+ raise CuminError('Unable to read configuration file:
{message}'.format(message=e))
+ except yaml.parser.ParserError as e:
+ raise CuminError("Unable to parse configuration file
'{config}':\n{message}".format(
+ config=config_file, message=e))
+
+ if config is None:
+ raise CuminError("Empty configuration found in
'{config}'".format(config=config_file))
+
+ return config
diff --git a/cumin/cli.py b/cumin/cli.py
index ae88a28..5bc038b 100644
--- a/cumin/cli.py
+++ b/cumin/cli.py
@@ -12,7 +12,6 @@
from logging.handlers import RotatingFileHandler # pylint:
disable=ungrouped-imports
import colorama
-import yaml
from ClusterShell.NodeSet import NodeSet
from tqdm import tqdm
@@ -171,27 +170,6 @@
logger.setLevel(logging.DEBUG)
else:
logger.setLevel(logging.INFO)
-
-
-def parse_config(config_file):
- """Parse the YAML configuration file.
-
- Arguments:
- config_file -- the path of the configuration file to load
- """
- try:
- with open(config_file, 'r') as f:
- config = yaml.safe_load(f)
- except IOError as e:
- raise cumin.CuminError('Unable to read configuration file:
{message}'.format(message=e))
- except yaml.parser.ParserError as e:
- raise cumin.CuminError("Unable to parse configuration file
'{config}':\n{message}".format(
- config=config_file, message=e))
-
- if config is None:
- raise cumin.CuminError("Empty configuration found in
'{config}'".format(config=config_file))
-
- return config
def sigint_handler(*args): # pylint: disable=unused-argument
@@ -375,7 +353,7 @@
return 0
user = get_running_user()
- config = parse_config(args.config)
+ config = cumin.Config(args.config)
setup_logging(config['log_file'], debug=args.debug)
except cumin.CuminError as e:
stderr(e)
diff --git a/cumin/tests/unit/test_cli.py b/cumin/tests/unit/test_cli.py
index cd9ace9..d7606fd 100644
--- a/cumin/tests/unit/test_cli.py
+++ b/cumin/tests/unit/test_cli.py
@@ -1,7 +1,4 @@
"""CLI tests."""
-import os
-import tempfile
-
from logging import DEBUG, INFO
import mock
@@ -78,32 +75,6 @@
cli.setup_logging('filename', debug=True)
logging.setLevel.assert_called_with(DEBUG)
assert file_handler.called
-
-
-def test_parse_config_ok():
- """The configuration file is properly parsed and accessible."""
- config = cli.parse_config('doc/examples/config.yaml')
- assert 'log_file' in config
-
-
-def test_parse_config_non_existent():
- """A CuminError is raised if the configuration file is not available."""
- with pytest.raises(CuminError, match='Unable to read configuration file'):
- cli.parse_config('not_existent_config.yaml')
-
-
-def test_parse_config_invalid():
- """A CuminError is raised if the configuration cannot be parsed."""
- invalid_yaml = '\n'.join((
- 'foo:',
- ' bar: baz',
- ' - foobar',
- ))
- tmpfile, tmpfilepath = tempfile.mkstemp(suffix='config.yaml',
prefix='cumin', text=True)
- os.write(tmpfile, invalid_yaml)
-
- with pytest.raises(CuminError, match='Unable to parse configuration file'):
- cli.parse_config(tmpfilepath)
@mock.patch('cumin.cli.stderr')
diff --git a/cumin/tests/unit/test_init.py b/cumin/tests/unit/test_init.py
new file mode 100644
index 0000000..12a9608
--- /dev/null
+++ b/cumin/tests/unit/test_init.py
@@ -0,0 +1,51 @@
+"""Cumin package tests."""
+import os
+import tempfile
+
+import pytest
+
+import cumin
+
+
+def test_config_class_instantiation():
+ """Should return the config. Multiple Config with the same path should
return the same object."""
+ config1 = cumin.Config('doc/examples/config.yaml')
+ assert 'log_file' in config1
+ config2 = cumin.Config('doc/examples/config.yaml')
+ assert config1 is config2
+
+
+def test_parse_config_ok():
+ """The configuration file is properly parsed and accessible."""
+ config = cumin.parse_config('doc/examples/config.yaml')
+ assert 'log_file' in config
+
+
+def test_parse_config_non_existent():
+ """A CuminError is raised if the configuration file is not available."""
+ with pytest.raises(cumin.CuminError, match='Unable to read configuration
file'):
+ cumin.parse_config('not_existent_config.yaml')
+
+
+def test_parse_config_invalid():
+ """A CuminError is raised if the configuration cannot be parsed."""
+ invalid_yaml = '\n'.join((
+ 'foo:',
+ ' bar: baz',
+ ' - foobar',
+ ))
+ tmpfile, tmpfilepath = tempfile.mkstemp(suffix='config.yaml',
prefix='cumin', text=True)
+ os.write(tmpfile, invalid_yaml)
+
+ with pytest.raises(cumin.CuminError, match='Unable to parse configuration
file'):
+ cumin.parse_config(tmpfilepath)
+
+
+def test_parse_config_empty():
+ """A CuminError is raised if the configuration is empty."""
+ empty_yaml = ''
+ tmpfile, tmpfilepath = tempfile.mkstemp(suffix='config.yaml',
prefix='cumin', text=True)
+ os.write(tmpfile, empty_yaml)
+
+ with pytest.raises(cumin.CuminError, match='Empty configuration found in'):
+ cumin.parse_config(tmpfilepath)
--
To view, visit https://gerrit.wikimedia.org/r/363746
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I924f0731268e15249be30e5b5482918f2d853265
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <[email protected]>
Gerrit-Reviewer: Faidon Liambotis <[email protected]>
Gerrit-Reviewer: Gehel <[email protected]>
Gerrit-Reviewer: Giuseppe Lavagetto <[email protected]>
Gerrit-Reviewer: Muehlenhoff <[email protected]>
Gerrit-Reviewer: Volans <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
