diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index 91a24549a..cd5637020 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -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 @@ def get(self, *args, **kwargs): 'logfile': '', 'loglevel': '', 'logger_obj': None, - 'logger': None, 'gdp_logger_obj': None, 'gdp_logger': None, 'auth_logger_obj': None, @@ -757,6 +757,13 @@ def __init__(self, config_file, verbose=False, skip_log=False, # 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 @@ def __init__(self, config_file, verbose=False, skip_log=False, 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 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, _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 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, # 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") diff --git a/mig/shared/logger.py b/mig/shared/logger.py index 5b949fca0..a2244f511 100644 --- a/mig/shared/logger.py +++ b/mig/shared/logger.py @@ -67,6 +67,23 @@ def _name_to_format(name): 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.""" diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json index 2d4edb02c..54b3d9673 100644 --- a/tests/fixture/mig_shared_configuration--new.json +++ b/tests/fixture/mig_shared_configuration--new.json @@ -104,8 +104,6 @@ ], "log_dir": "", "logfile": "", - "logger": null, - "logger_obj": null, "loglevel": "", "lrmstypes": [], "mig_code_base": "", diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index 7c9aef06e..655739fcd 100644 --- a/tests/test_mig_shared_configuration.py +++ b/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.