refactor: also error on missing file or invalid extension

Refactored the code to reuse check that logging config file is set and
that the file exists.

Now throws error and exits if file name does not end in .ini or .json.

Now throws error if file doesn't exist. Before it would just configure
default logging as though file wasn't specified.

Added tests for these two cases.

Author: rouilj

roundup/configuration.py

@@ -2392,45 +2392,56 @@ def load_config_dict_from_json_file(self, filename):
 
     def init_logging(self):
         _file = self["LOGGING_CONFIG"]
-        if _file and os.path.isfile(_file) and _file.endswith(".ini"):
-            logging.config.fileConfig(
-                _file,
-                disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"])
-            return
-
-        if _file and os.path.isfile(_file) and _file.endswith(".json"):
-            config_dict = self.load_config_dict_from_json_file(_file)
-            try:
-                logging.config.dictConfig(config_dict)
-            except ValueError as e:
-                # docs say these exceptions:
-                #    ValueError, TypeError, AttributeError, ImportError
-                # could be raised, but
-                # looking through the code, it looks like
-                # configure() maps all exceptions (including
-                # ImportError, TypeError) raised by functions to
-                # ValueError.
-                context = "No additional information available."
-                if hasattr(e, '__context__') and e.__context__:
-                    # get additional error info. E.G. if INFO
-                    # is replaced by MANGO, context is:
-                    #    ValueError("Unknown level: 'MANGO'")
-                    # while str(e) is "Unable to configure handler 'access'"
-                    context = e.__context__
-
-                raise LoggingConfigError(
-                    'Error loading logging dict from %(file)s.\n'
-                    '%(msg)s\n%(context)s\n' % {
-                        "file": _file,
-                        "msg": type(e).__name__ + ": " + str(e),
-                        "context": context
-                    },
-                    config_file=self.filepath,
-                    source="dictConfig"
-                )
-
+        if _file and os.path.isfile(_file):
+            if _file.endswith(".ini"):
+                logging.config.fileConfig(
+                    _file,
+                    disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"])
+            elif _file.endswith(".json"):
+                config_dict = self.load_config_dict_from_json_file(_file)
+                try:
+                    logging.config.dictConfig(config_dict)
+                except ValueError as e:
+                    # docs say these exceptions:
+                    #    ValueError, TypeError, AttributeError, ImportError
+                    # could be raised, but
+                    # looking through the code, it looks like
+                    # configure() maps all exceptions (including
+                    # ImportError, TypeError) raised by functions to
+                    # ValueError.
+                    context = "No additional information available."
+                    if hasattr(e, '__context__') and e.__context__:
+                        # get additional error info. E.G. if INFO
+                        # is replaced by MANGO, context is:
+                        #    ValueError("Unknown level: 'MANGO'")
+                        # while str(e) is "Unable to configure handler 'access'"
+                        context = e.__context__
+
+                    raise LoggingConfigError(
+                        'Error loading logging dict from %(file)s.\n'
+                        '%(msg)s\n%(context)s\n' % {
+                            "file": _file,
+                            "msg": type(e).__name__ + ": " + str(e),
+                            "context": context
+                        },
+                        config_file=self.filepath,
+                        source="dictConfig"
+                    )
+            else:
+                raise OptionValueError(
+                    self.options['LOGGING_CONFIG'],
+                    _file,
+                    "Unable to load logging config file. "
+                    "File extension must be '.ini' or '.json'.\n"
+                    )
+                
             return
 
+        if _file:
+            raise OptionValueError(self.options['LOGGING_CONFIG'],
+                                   _file,
+                                   "Unable to find logging config file.")
+        
         _file = self["LOGGING_FILENAME"]
         # set file & level on the roundup logger
         logger = logging.getLogger('roundup')

test/test_config.py

@@ -1370,3 +1370,41 @@ def testDictLoggerConfigViaJson(self):
                   "%s'\n" % (log_config_filename, access_filename))
         self.assertEqual(output, target)
 
+    def test_missing_logging_config_file(self):
+        saved_config = self.db.config['LOGGING_CONFIG']
+        
+        self.db.config['LOGGING_CONFIG'] = 'logging.json'
+        
+        with self.assertRaises(configuration.OptionValueError) as cm:
+            self.db.config.init_logging()
+
+        self.assertEqual(cm.exception.args[1], "_test_instance/logging.json")
+        self.assertEqual(cm.exception.args[2],
+                         "Unable to find logging config file.")
+
+        self.db.config['LOGGING_CONFIG'] = 'logging.ini'
+
+        with self.assertRaises(configuration.OptionValueError) as cm:
+            self.db.config.init_logging()
+
+        self.assertEqual(cm.exception.args[1], "_test_instance/logging.ini")
+        self.assertEqual(cm.exception.args[2],
+                         "Unable to find logging config file.")
+
+        self.db.config['LOGGING_CONFIG'] = saved_config
+        
+    def test_unknown_logging_config_file_type(self):
+        saved_config = self.db.config['LOGGING_CONFIG']
+
+        self.db.config['LOGGING_CONFIG'] = 'schema.py'
+
+        
+        with self.assertRaises(configuration.OptionValueError) as cm:
+            self.db.config.init_logging()
+
+        self.assertEqual(cm.exception.args[1], "_test_instance/schema.py")
+        self.assertEqual(cm.exception.args[2],
+                         "Unable to load logging config file. "
+                         "File extension must be '.ini' or '.json'.\n")
+
+        self.db.config['LOGGING_CONFIG'] = saved_config