Avoid linting failures by unconditionally defining a logger property.

mig/shared/configuration.py

@@ -1,3 +1,3 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
@@ -37,6 +37,7 @@
 import copy
 import datetime
 import functools
+import inspect
 import os
 import pwd
 import re
@@ -67,7 +68,7 @@
         generic_valid_days, DEFAULT_USER_ID_FORMAT, valid_user_id_formats, \
         valid_filter_methods, default_twofactor_auth_apps, \
         mig_conf_section_dirname
-    from mig.shared.logger import Logger, SYSLOG_GDP
+    from mig.shared.logger import Logger, BareLoggerAdapter, SYSLOG_GDP
     from mig.shared.htmlgen import menu_items, vgrid_items
     from mig.shared.fileio import read_file, load_json, write_file
 except ImportError as ioe:
@@ -693,7 +694,6 @@
     'logfile': '',
     'loglevel': '',
     'logger_obj': None,
-    'logger': None,
     'gdp_logger_obj': None,
     'gdp_logger': None,
     'auth_logger_obj': None,
@@ -757,6 +757,13 @@
         # Explicitly init a few helpers hot-plugged and used in ways where it's
         # less obvious if they are always guaranteed to already be initialized.
         self.default_page = None
+
+        # internal state
+        self._loaded = False
+
+        # logging related
+        self.logger_obj = None
+        self._logger = None
         self.auth_logger_obj = None
         self.gdp_logger_obj = None
 
@@ -770,6 +777,34 @@
                                disable_auth_log=disable_auth_log,
                                _config_file=config_file)
 
+    @property
+    def logger(self):
+        assert self._logger, "logging attempt prior to logger availability"
+        return self._logger
+
+    @logger.setter
+    def logger(self, logger):
+        """Setter method that correctly sets logger related properties."""
+
+        # attempt to determine what type of objetc we were given - this logic
+        # exists to deal with some fallout from having both logger_obj and
+        # logger properties and which of them should be set
+
+        if inspect.ismethod(getattr(logger, 'reopen', None)):
+            # we have a logger_obj, not a plain logger
+            # record it that way to ensure it could be corrctly reopened where
+            # otherwise refefences to the object that has it may be lost and it
+            # may not occur
+            self.logger_obj = logger
+            self._logger = logger.logger
+        elif inspect.ismethod(getattr(logger, 'info', None)):
+            # we have a bare logger object based on the sanity check
+            self._logger = logger
+            self.logger_obj = BareLoggerAdapter(logger)
+        else:
+            raise AssertionError("attempted assignment of unsupported logger")
+
+
     def reload_config(self, verbose, skip_log=False, disable_auth_log=False,
                       _config_file=None):
         """Re-read and parse configuration file. Optional skip_log arg
@@ -786,12 +821,6 @@
         _config_file = _config_file or self.config_file
         assert _config_file is not None
 
-        try:
-            if self.logger:
-                self.logger.info('reloading configuration and reopening log')
-        except:
-            pass
-
         try:
             config_file_is_path = os.path.isfile(_config_file)
         except TypeError:
@@ -841,13 +870,17 @@
 
         # reopen or initialize logger
 
-        if self.logger_obj:
+        if self._loaded:
+            self.logger.info('reloading configuration and reopening log')
             self.logger_obj.reopen()
         else:
-            self.logger_obj = Logger(self.loglevel, logfile=self.log_path)
+            self.logger = Logger(self.loglevel, logfile=self.log_path)
+            self.logger.info('loading configuration and opening log')
+
+            # record that the object has been populated
+            self._loaded = True
 
-        logger = self.logger_obj.logger
-        self.logger = logger
+        logger = self.logger
 
         # print "logger initialized (level " + logger_obj.loglevel() + ")"
         # logger.debug("logger initialized")

mig/shared/logger.py

@@ -39,7 +39,7 @@
 _default_level = "info"
 _default_format = "%(asctime)s %(levelname)s %(message)s"
 _debug_format = "%(asctime)s %(module)s:%(funcName)s:%(lineno)s %(levelname)s %(message)s"
 __hangup_helpers = {}
 
 SYSLOG_GDP = syslog.LOG_LOCAL0
 
@@ -67,6 +67,23 @@
     return formats[name]
 
 
+class BareLoggerAdapter:
+    """Small wrapper to adapt an arbitrary bare logger to the MiG Logger API"""
+
+    def __init__(self, logger):
+        self._logger = logger
+
+    @property
+    def logger(self):
+        return self._logger
+
+    def reopen(self):
+        pass
+
+    def shutdown(self):
+        pass
+
+
 class SysLogLibHandler(logging.Handler):
     """A logging handler that emits messages to syslog.syslog."""
 
@@ -292,10 +309,10 @@
         Logger("INFO", logfile=log_path, app="testing")
     conf.logger = logger = logger_obj.logger
     print("Add some log entries")
     logger.debug("for unit testing")
     logger.info("for unit testing")
     logger.warning("for unit testing")
     logger.error("for unit testing")
     print("Now log contains:")
     log_fd = open(log_path, "r")
     for line in log_fd:
@@ -305,7 +322,7 @@
     os.remove(log_path)
     logger_obj.reopen()
     print("Add another log entry")
     logger.info("for unit testing")
     print("Now log contains:")
     log_fd = open(log_path, "r")
     for line in log_fd:

tests/fixture/mig_shared_configuration--new.json

@@ -104,8 +104,6 @@
   ],
   "log_dir": "",
   "logfile": "",
-  "logger": null,
-  "logger_obj": null,
   "loglevel": "",
   "lrmstypes": [],
   "mig_code_base": "",

tests/test_mig_shared_configuration.py

@@ -34,6 +34,7 @@
 from tests.support import MigTestCase, TEST_DATA_DIR, PY2, testmain, \
     fixturefile
 from mig.shared.configuration import Configuration
+from mig.shared.logger import null_logger
 
 
 def _is_method(value):
@@ -42,7 +43,7 @@ def _is_method(value):
 
 def _to_dict(obj):
     return {k: v for k, v in inspect.getmembers(obj)
-            if not (k.startswith('__') or _is_method(v))}
+            if not (k.startswith('_') or k.startswith('logger') or _is_method(v))}
 
 
 class MigSharedConfiguration(MigTestCase):
@@ -320,6 +321,9 @@ def test_default_object(self):
             'mig_shared_configuration--new', fixture_format='json')
 
         configuration = Configuration(None)
+        # attach a null logger to sidestep the useful logger before available
+        # assertion which would otherwise blow when the object is inspected
+        configuration.logger = null_logger("test_configuration")
         # TODO: the following work-around default values set for these on the
         #       instance that no longer make total sense but fiddling with them
         #       is better as a follow-up.