slight rework cfg module, moving file validation and logic into setters. slight improvement on logging and tests

app_skellington/_bootstrap.py

@@ -23,14 +23,19 @@ _log_fmt = '%(levelname)-7s:%(message)s'
 _logger_name = 'app_skellington'
 _bootstrap_logger = logging.getLogger(_logger_name)
 
-# Logging is manually switched on via environment variable:
-if os.environ.get('APP_SKELLINGTON_DEBUG', None):
+
+# NOTE(MG) This is done twice: once when app_skellington
+# module is imported via _bootstrap.py and again if logging
+# configuration is reloaded. This catches APPSKELLINGTON_DEBUG
+# environment variable the first time, as app_skellington module
+# is imported. See cfg.py
+if os.environ.get('APPSKELLINGTON_DEBUG', None):
     _bootstrap_logger.setLevel(logging.DEBUG) # Don't filter any log levels
     fmt = logging.Formatter(_log_fmt)
     handler = logging.StreamHandler()
     handler.setFormatter(fmt)
     _bootstrap_logger.addHandler(handler)
-    _bootstrap_logger.debug('debug log enabled: APP_SKELLINGTON_DEBUG set in environment variables')
+    _bootstrap_logger.debug('log - APPSKELLINGTON_DEBUG set in environment: Enabling verbose logging.')
 
 # Logging is by default off, excepting CRITICAL
 else:   

app_skellington/app_container.py

@@ -48,20 +48,19 @@ class ApplicationContainer:
     def __init__(
         self,
         configspec_filepath=None,
-        config_filepath=None,
+        configini_filepath=None,
         *args, **kwargs
     ):
         # Instantiate root application context (container for globals)
-        if configspec_filepath is None:
-            configspec_filepath = self._get_configspec_filepath()
+        # if configspec_filepath is None:
+        #     configspec_filepath = self._get_configspec_filepath()
 
         self.appname = kwargs.get('appname') or DEFAULT_APP_NAME 
         self.appauthor = kwargs.get('appauthor') or DEFAULT_APP_AUTHOR
 
         self._dependencies = {}
 
-        config = cfg.Config(configspec_filepath)
-        config.load_config_from_file(config_filepath)
+        config = cfg.Config(configspec_filepath, configini_filepath)
 
         logger = log.LoggingLayer(self.appname, self.appauthor)
 

app_skellington/cfg.py

@@ -1,3 +1,9 @@
+# The cfg module provides Config, the app_skellington service for interfacing
+# config.ini files, environment variables (*todo), and
+# application-wide configuration. The underlying mechanism is built on top of
+# ConfigObj module and it's recommended to use config.spec files to define
+# your available configuration of the relevant application.
+
 from . import _util
 from ._bootstrap import _bootstrap_logger
 
@@ -18,15 +24,99 @@ class Config:
         'allow_options_beyond_spec': True,
     }
 
-    def __init__(self, configspec_filepath=None, capabilities=None):
-        self.config_obj = None # Reference to configobj.ConfigObj()
-        self._config_filepaths = []
+    def __init__(
+        self,
+        configspec_filepath=None,
+        configini_filepath=None,
+        capabilities=None
+    ):
+        self._config_obj = None # must be type configobj.ConfigObj()
+        self._configini_data = None
+        self._configini_filepath = None
+        self._configspec_data = None
         self._configspec_filepath = None
-        # self.configspec_filepath = configspec_filepath
+        self._has_accessed_internal_configobj = True
+        self._has_changed_internally = True
+        self._capability_enforce_strict_spec_validation = False
+
+        # Register the files and parse provided config.spec and config.ini
+        self.configspec_filepath = configspec_filepath
+        self.configini_filepath = configini_filepath
+        # NOTE(MG) Setters above trigger load_config()
+
+    @property
+    def config_obj(self):
+        self._has_accessed_internal_configobj = True
+        return self._config_obj
+
+    # config_filepath:
+    ###########################################################################
+    @property
+    def configini_filepath(self):
+        """
+        Config.ini filepath of site-specified configuration settings. File
+        stored on user machine. Reloads config when set.
+        """
+        return self._configini_filepath
+
+    @configini_filepath.setter
+    def configini_filepath(self, value):
+        self._configini_filepath = value
+        self._has_changed_internally = True
+        self.load_config()
+
+    # configspec_filepath:
+    ###########################################################################
+    @property
+    def configspec_filepath(self):
+        """
+        Config.spec filepath of site-specified configuration settings. File
+        stored in module directory. Reloads config when set.
+        """
+        return self._configspec_filepath
+
+    @configspec_filepath.setter
+    def configspec_filepath(self, filepath):
+        # Check if exists as file (at least seems to):
+        # if not _util.does_file_exist(filepath):
+        #     _bootstrap_logger.error(
+        #         'failed to set config.spec: file not found '
+        #         '(%s)', filepath)
+        #     raise Exception
+        if filepath is None:
+            _bootstrap_logger.debug(
+                'cfg - clearing configspec'
+            )
+            self._configspec_filepath = None
+            self._configspec_data = None
+            self._has_changed_internally = True
+            self.load_config()
+            return
+
+        try:
+            with open(filepath) as fp:
+                data = fp.read()
+                self._configspec_filepath = filepath
+                self._configspec_data = data
+                self._has_changed_internally = True
+                _bootstrap_logger.debug(
+                    'cfg - set configspec and read contents: %s',
+                    filepath
+                )
+                self.load_config()
+                return
+        except OSError as ex:
+            _bootstrap_logger.critical(
+                'cfg - failed to find config.spec: file not found (%s)',
+                filepath
+            )
+            raise OSError('failed to read provided config.spec file')
+
+        self.load_config()
 
     def __contains__(self, key):
         try:
-            has_item = key in self.config_obj
+            has_item = key in self._config_obj
             return has_item
         except KeyError as ex:
             pass
@@ -46,9 +136,13 @@ class Config:
         Returns the value of the configuration item identified by <key>.
         """
         try:
-            return self.config_obj[key].dict()
+            v = self._config_obj[key]
+            if isinstance(v, str):
+                return v
+            else:
+                # return self._config_obj[key].dict()
+                return self._config_obj[key]
         except KeyError as ex:
-            # raise ConfigurationItemNotFoundError()
             raise
 
     def __setitem__(self, key, value):
@@ -56,135 +150,103 @@ class Config:
         Assigns the value of the configuration item
         identified by <key> as <value>.
         """
-        self[key] = value
+        self._config_obj[key] = value
 
-    @property
-    def config_filepath(self, idx=0):
-        """
-        Returns the config filepath (optionally specified by index
-        when using multiple config files).
-        """
-        assert idx>=0, 'invalid idx argument: index must be greater than 0'
-        if len(self._config_filepaths) > 0:
-            try:
-                return self._config_filepaths[idx]
-            except ValueError as ex:
-                return
+    def load_config(
+        self, configspec_filepath=None, configini_filepath=None
+    ):
+        # Set new arguments if were passed in:
+        if configspec_filepath is not None:
+            self.configspec_filepath = configspec_filepath
+        if configini_filepath is not None:
+            self.configini_filepath = configini_filepath
 
-    @config_filepath.setter
-    def config_filepath(self, value, idx=0):
-        """
-        Assigns <value> as the config filepath (optionally specified by index
-        when using multiple config files).
-        """
-        assert idx>=0, 'invalid idx argument: index must be greater than 0'
-        self._config_filepaths[idx] = value
-
-    @property
-    def configspec_filepath(self):
-        return self._configspec_filepath
-
-    @configspec_filepath.setter
-    def configspec_filepath(self, filepath):
-        if _util.does_file_exist(filepath):
-            self._configspec_filepath = filepath
-        else:
-            _bootstrap_logger.error(
-                'failed to set config.spec: file not found '
-                '(%s)', filepath)
-
-    def load_config_from_file(self, config_filepath):
-        """
-        Loads configuration settings from file, overwritting all configuration.
-        """
-        # Record all config.ini files passed in
-        if config_filepath not in self._config_filepaths:
-            self._config_filepaths.append(config_filepath)
-
-        # Check for config.spec
+        # Load and validate configuration if possible:
+        self._load_config_files()
         if self.configspec_filepath:
-            _bootstrap_logger.info('using config.spec: %s', self.configspec_filepath)
-        else:
-            _bootstrap_logger.info('config.spec not defined')
-        _bootstrap_logger.info('using config file: %s', config_filepath)
+            rc = self._validate_config_against_spec()
+            if not rc:
+                if self._capability_enforce_strict_spec_validation:
+                    raise RuntimeError('Failed to validate config.ini against spec.')
+                return False
+        return True
 
-        # Pre-check for config.ini existence 
-        if _util.does_file_exist(config_filepath):
-            _bootstrap_logger.info('existing config file found')
-        else:
-            _bootstrap_logger.info('no config file found: using defaults')
+    def _load_config_files(self):
+        config_spec = self.configspec_filepath
+        config_ini = self.configini_filepath
 
         # interpolation='template' changes config file variable replacement to
         # use the form $var instead of %(var)s, which is useful to enable
         # literal %(text)s values in the config.
         try:
-            configspec_filepath = self.configspec_filepath
-            if configspec_filepath:
-                self.config_obj = configobj.ConfigObj(
-                    config_filepath,
-                    configspec=configspec_filepath,
-                    interpolation='template'
-                )
-            else:
-                self.config_obj = configobj.ConfigObj(
-                    config_filepath,
-                    # configspec=configspec_filepath,
-                    interpolation='template'
-                )
+            self._config_obj = configobj.ConfigObj(
+                infile=config_ini,
+                # options
+                configspec=config_spec,
+                # encoding
+                interpolation='template'
+                # raise_errors
+            )
+            _bootstrap_logger.debug(
+                'cfg - Parsed configuration. config.spec = %s, config.ini = %s',
+                config_spec, config_ini
+            )
+            return True
+
         except configobj.ParseError as ex:
-            msg = 'failed to load config: error in config.spec configuration: {}'.format(config_filepath)
+            msg = 'cfg - Failed to load config: error in config.spec configuration: {}'.format(config_filepath)
             _bootstrap_logger.error(msg)
-            _util.eprint(msg)
             return False
         except OSError as ex:
-            msg = 'failed to load config: config.spec file not found'
+            msg = 'cfg - Failed to load config: config.spec file not found.'
             _bootstrap_logger.error(msg)
-            _util.eprint(msg)
             return False
+        except Exception as ex:
+            print(ex)
 
+    def _validate_config_against_spec(self):
+        config_spec = self.configspec_filepath
+        config_ini = self.configini_filepath
 
         # Hack the configobj module to alter the interpolation for validate.py:
         configobj.DEFAULT_INTERPOLATION = 'template'
-        self.config_obj.filename = config_filepath
 
-        if self.configspec_filepath:
-            # Validate config.ini against config.spec
-            try:
-                _bootstrap_logger.info('validating config file against spec')
-                val = validate.Validator()
-                test_results = self.config_obj.validate(
-                    val, copy=True, preserve_errors=True
+        # Validate config.ini against config.spec
+        try:
+            _bootstrap_logger.info('cfg - Validating config file against spec')
+            val = validate.Validator()
+            assert isinstance(self._config_obj, configobj.ConfigObj), 'expecting configobj.ConfigObj, received %s' % type(self._config_obj)
+            # NOTE(MG) copy below instructs configobj to use defaults from spec file
+            test_results = self._config_obj.validate(
+                val, copy=True, preserve_errors=True
+            )
+            if test_results is True:
+                _bootstrap_logger.info(
+                    'cfg- Successfully validated configuration against spec. input = %s, validation spec = %s',
+                    config_ini, config_spec
                 )
-                if test_results is True:
-                    _bootstrap_logger.info(
-                        'application configuration file passed validation. input = %s, validation spec = %s',
-                        config_filepath, configspec_filepath
-                        )
+                return True
 
-                else:
-                    _bootstrap_logger.critical('config file failed validation')
+            elif test_results is False:
+                _bootstrap_logger.debug(
+                    'cfg - Potentially discovered invalid config.spec'
+                )
 
-                    for (section_list, key, rslt) in configobj.flatten_errors(self.config_obj, test_results):
-                        _bootstrap_logger.critical('config error info: %s %s %s', section_list, key, rslt)
-                        if key is not None:
-                            _bootstrap_logger.critical('config failed validation: [%s].%s appears invalid. msg = %s', '.'.join(section_list), key, rslt)
-                        else:
-                            _bootstrap_logger.critical("config failed validation: missing section, name = '%s'. msg = %s", '.'.join(section_list), rslt)
-                    return False
-            except ValueError as ex:
-                _bootstrap_logger.error('failed validating configspec')
+            else:
+                self._validate_parse_errors(test_results)
                 return False
+        except ValueError as ex:
+            _bootstrap_logger.error('cfg - Failed while validating config against spec. ')
+            return False
 
-        # Create the config file if it doesn't exist
-        # if not _util.does_file_exist(config_filepath):
-        if True:
-            _bootstrap_logger.info('writing new config file: %s', config_filepath)
-            dirname = os.path.dirname(config_filepath)
-            _util.ensure_dir_exists(dirname)
-            self.config_obj.write()
-
-        _bootstrap_logger.info('done loading config file')
-        return True
+    def _validate_parse_errors(self, test_results):
+        _bootstrap_logger.critical('cfg - Config file failed validation.')
+        for (section_list, key, rslt) in configobj.flatten_errors(self._config_obj, test_results):
+            _bootstrap_logger.critical('cfg - Config error info: %s %s %s', section_list, key, rslt)
+            if key is not None:
+                _bootstrap_logger.critical('cfg - Config failed validation: [%s].%s appears invalid. msg = %s', '.'.join(section_list), key, rslt)
+            else:
+                _bootstrap_logger.critical("cfg - Config failed validation: missing section, name = '%s'. msg = %s", '.'.join(section_list), rslt)
 
     def print_config(self):
         """
@@ -192,16 +254,13 @@ class Config:
         """
         print('config:')
 
-        self.config_obj.walk(print)
-        for section in self.config_obj.sections:
+        self._config_obj.walk(print)
+        for section in self._config_obj.sections:
             print(section)
-            for key in self.config_obj[section]:
-                print('    ',  self.config_obj[section][key])
+            for key in self._config_obj[section]:
+                print('    ',  self._config_obj[section][key])
 
 class EnvironmentVariables:
     def __init__(self):
         raise NotImplementedError
 
-class ConfigurationItemNotFoundError(Exception):
-    pass
-

app_skellington/cli.py

@@ -153,7 +153,8 @@ class CommandTree:
                     key,
                     help=helptext,
                     nargs='?',
-                    default=param.default)
+                    default=param.default
+                )
             else:
                 helptext = 'required'
                 self.root_parser.add_argument(
@@ -219,20 +220,20 @@ class CommandTree:
     def run_command(self, args=None):
         args, unk, success = self.parse(args)
         if not success:
-            _bootstrap_logger.info('SystemExit: Perhaps user invoked --help')
+            _bootstrap_logger.info('cli - SystemExit: Perhaps user invoked --help')
             return
 
         if args is False and unk is False:
             _bootstrap_logger.error('failed parsing args')
             return False
-        _bootstrap_logger.info('received args from shell: %s', args)
+        _bootstrap_logger.info('cli - received args from shell: %s', args)
 
         args = vars(args)
 
         cmd = self._lookup_command(args)
         if cmd is None:
             print('cmd is None')
-            _bootstrap_logger.error('failed to find command')
+            _bootstrap_logger.error('cli - failed to find command')
             return False
 
         return self._invoke_command(cmd, args)
@@ -265,10 +266,10 @@ class CommandTree:
                     input('<broken>')
 
                 val = args.get(argparse_param)
-                _bootstrap_logger.debug('argparse command is \'{}\' = {}'.format(argparse_param, val))
+                _bootstrap_logger.debug('cli - argparse command is \'{}\' = {}'.format(argparse_param, val))
 
                 lookup = submenu.entries.get(val)
-                _bootstrap_logger.debug('lookup, entries[{}] = {}'.format(val, lookup))
+                _bootstrap_logger.debug('cli - lookup, entries[{}] = {}'.format(val, lookup))
                 # print(submenu.entries)
 
                 # pop value
@@ -295,8 +296,8 @@ class CommandTree:
             if param.name in args:
                 func_args.append(args[param.name])
 
-        _bootstrap_logger.info('function: %s', func)
-        _bootstrap_logger.info('function args: %s', func_args)
+        _bootstrap_logger.info('cli - function: %s', func)
+        _bootstrap_logger.info('cli - function args: %s', func_args)
         return command_to_be_invoked(*func_args)
 
     def _get_subparser(self):
@@ -394,12 +395,14 @@ class SubMenu:
                     key,
                     help=helptext,
                     nargs='?',
-                    default=param.default)
+                    default=param.default
+                )
             else:
                 helptext = 'required'
                 child_node.add_argument(
                     key,
-                    help=helptext)
+                    help=helptext
+                )
 
         # # Wrapper function that instantiates an object and runs a method
         # # on-demand. The object is created, injected with necessary
@@ -419,7 +422,7 @@ class SubMenu:
         registered_name = '{}.{}'.format(
             self.submenu_path,
             cmd_name)
-        _bootstrap_logger.info('registered command: %s', registered_name)
+        _bootstrap_logger.info('cli - registered command: %s', registered_name)
         self.entries[cmd_name] = cmd
 
     def create_submenu( 
@@ -471,7 +474,7 @@ class SubMenu:
         submenu.submenu_path = '{}.{}'.format(self.submenu_path, cmd_entry_name)
         submenu_name = submenu.submenu_path
 
-        _bootstrap_logger.info('registered submenu: %s', submenu_name)
+        _bootstrap_logger.info('cli - registered submenu: %s', submenu_name)
         self.entries[cmd_entry_name] = submenu
         return submenu
 

app_skellington/log.py

@@ -70,16 +70,16 @@ class LoggingLayer:
         noise for typical operation.
         """
         if config_dict is None:
-            _bootstrap_logger.debug('No application logging configuration provided. Using default')
+            _bootstrap_logger.debug('log - No application logging configuration provided. Using default')
             config_dict = DEFAULT_LOG_SETTINGS
 
         self.transform_config(config_dict)
 
         try:    
             # TODO(MG) switch to pretty-print, as it'd be more human readable
-            _bootstrap_logger.debug('Log configuration: %s', config_dict)
+            _bootstrap_logger.debug('log - Log configuration: %s', config_dict)
             logging.config.dictConfig(config_dict)
-            _bootstrap_logger.debug('Configured all logging')
+            _bootstrap_logger.debug('log - Configured all logging')
         except Exception as ex:
             print('unable to configure logging:', ex, type(ex))
 
@@ -127,7 +127,13 @@ class LoggingLayer:
             config_dict['handlers']['file']['filename'] = log_filepath
 
     def _add_own_logconfig(self, config_dict):
-        if os.environ.get('APP_SKELLINGTON_DEBUG', None):
+        # NOTE(MG) This is done twice: once when app_skellington
+        # module is imported again if logging configuration is
+        # reloaded. This catches APPSKELLINGTON_DEBUG environment
+        # variable the second time, when it's being reloaded as a
+        # logging configuration is read from config file.
+        # See _bootstrap.py
+        if os.environ.get('APPSKELLINGTON_DEBUG', None):
             if 'app_skellington' not in config_dict['loggers']:
                 config_dict['loggers']['app_skellington'] = {
                     'level': 'debug', 'propagate': 'false'

tests/README.md

@@ -0,0 +1,18 @@
+Software tests for app_skellington
+==================================
+Contained is the early stages of unit testing for app_skellington framework.
+
+Usage
+-----
+Run all tests (cwd is testing directory):
+
+    pytest .
+
+Run test by keyword:
+
+    pytest -k "<test keyword>" .
+
+Run test by directory:
+
+    pytest <dirname>
+  

tests/cfg/sample_config.ini

@@ -0,0 +1,5 @@
+root_option = root_option_val
+
+[app]
+    sub_option = sub_option_val
+

tests/cfg/sample_config.spec

@@ -0,0 +1,5 @@
+root_option = string(max=255, default='def_string')
+
+[app]
+    sub_option = string(max=255, default='def_sub')
+

tests/cfg/sample_config_full.ini

@@ -0,0 +1,5 @@
+root_option = root_option_val
+
+[app]
+    sub_option = sub_option_val
+

tests/cfg/sample_config_full.spec

@@ -0,0 +1,6 @@
+root_option = string(max=255, default='def_string')
+int_option = integer(min=0, max=100)
+
+[app]
+    sub_option = string(max=255, default='def_sub')
+

tests/cfg/sample_config_invalid.spec

@@ -0,0 +1,2 @@
+root_option = invalid(max=255, default='def_string')
+

tests/cfg/test_cfg.py

@@ -0,0 +1,97 @@
+from app_skellington.cfg import Config
+from app_skellington import _util
+
+import pytest
+
+@pytest.fixture
+def sample_configspec_filepath():
+        return _util.get_asset(__name__, 'sample_config.spec')
+
+@pytest.fixture
+def sample_configini_filepath():
+        return _util.get_asset(__name__, 'sample_config.ini')
+
+@pytest.fixture
+def sample_full_configspec_filepath():
+        return _util.get_asset(__name__, 'sample_config_full.spec')
+
+@pytest.fixture
+def sample_full_configini_filepath():
+        return _util.get_asset(__name__, 'sample_config_full.ini')
+
+@pytest.fixture
+def sample_invalid_configspec_filepath():
+        return _util.get_asset(__name__, 'sample_config_invalid.spec')
+
+class TestConfig_e2e:
+    def test_allows_reading_ini_and_no_spec(
+        self, sample_configini_filepath
+    ):
+        cfg = Config(
+            configini_filepath=sample_configini_filepath
+        )
+        assert cfg['root_option'] == 'root_option_val', 'expecting default from config.spec (didnt get)'
+        assert cfg['app']['sub_option'] == 'sub_option_val', 'expecting default for sub option'
+
+    def test_allows_reading_spec_and_no_ini(
+        self, sample_configspec_filepath
+    ):
+        cfg = Config(
+            configspec_filepath=sample_configspec_filepath
+        )
+        assert cfg['root_option'] == 'def_string', 'expecting default from config.spec (didnt get)'
+
+    # NOTE(MG) Changed the functionality to not do it this way.
+    # def test_constructor_fails_with_invalid_spec(
+    #     self, sample_invalid_configspec_filepath
+    # ):
+    #     with pytest.raises(Exception):
+    #         cfg = Config(
+    #             configspec_filepath=sample_invalid_configspec_filepath
+    #         )
+
+    def test_allows_options_beyond_spec(
+        self, sample_configspec_filepath
+    ):
+        cfg = Config(
+            configspec_filepath=sample_configspec_filepath
+        )
+        cfg['foo'] = 'test my value'
+        assert cfg['foo'] == 'test my value'
+
+        cfg['app']['bar'] = 'another value'
+        assert cfg['app']['bar'] == 'another value'
+
+    # def test_can_read_config_file_mutiple_times(self):
+    #     pass
+
+    def test_can_override_config_file_manually(
+        self, sample_configini_filepath
+    ):
+        cfg = Config(
+            configini_filepath=sample_configini_filepath
+        )
+        cfg['root_option'] = 'newval'
+        assert cfg['root_option'] == 'newval'
+
+        cfg['app']['sub_option'] = 'another_new_val'
+        assert cfg['app']['sub_option'] == 'another_new_val', 'expecting default for sub option'
+
+    def test_can_set_option_without_config(self):
+        cfg = Config()
+        cfg['foo'] = 'test my value'
+        assert cfg['foo'] == 'test my value'
+
+        cfg['app'] = {}
+        cfg['app']['bar'] = 'another value'
+        assert cfg['app']['bar'] == 'another value'
+
+    def test_uses_spec_as_defaults(
+        self, sample_configspec_filepath
+    ):
+        cfg = Config(
+            configspec_filepath=sample_configspec_filepath
+        )
+        assert cfg['root_option'] == 'def_string', 'expecting default from config.spec (didnt get)'
+        assert cfg['app']['sub_option'] == 'def_sub', 'expecting default for sub option'
+

tests/test_cfg.py

@@ -1,31 +0,0 @@
-from app_skellington.cfg import Config
-
-class TestConfig_e2e:
-    def test_allows_reading_with_no_spec(self):
-        x = Config()
-        assert True == False
-
-    def test_allows_reading_with_sample_spec(self):
-        x = Config()
-        assert True == False
-
-    def test_constructor_fails_with_invalid_spec(self):
-        x = Config()
-        assert True == False
-
-    def test_allows_options_beyond_spec(self):
-        x = Config()
-        assert True == False
-
-    def test_can_read_config_correctly_from_file(self):
-        pass
-
-    def test_can_read_config_file_mutiple_times(self):
-        pass
-
-    def test_can_override_config_file_manually(self):
-        pass
-
-    def test_can_set_option_without_config(self):
-        pass
-