Volans has uploaded a new change for review. ( 
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(-)


  git pull ssh://gerrit.wikimedia.org:29418/operations/software/cumin 
refs/changes/46/363746/1

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: newchange
Gerrit-Change-Id: I924f0731268e15249be30e5b5482918f2d853265
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <rcocci...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to