[trac1010] add logger config update call

Renamed the original my_logconfig_handler() to default_logconfig_handler and made it public. This is now called by the python wrapper function log_config_update (a module function in isc.log)

src/lib/config/ccsession.cc

@@ -247,7 +247,9 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
 } // end anonymous namespace
 
 void
-my_logconfig_handler(const std::string&n, ConstElementPtr new_config, const ConfigData& config_data) {
+default_logconfig_handler(const std::string&n,
+                          ConstElementPtr new_config,
+                          const ConfigData& config_data) {
     config_data.getModuleSpec().validateConfig(new_config, true);
 
     std::vector<isc::log::LoggerSpecification> specs;
@@ -353,7 +355,7 @@ ModuleCCSession::ModuleCCSession(
 
     // Keep track of logging settings automatically
     if (handle_logging) {
-        addRemoteConfig("Logging", my_logconfig_handler, false);
+        addRemoteConfig("Logging", default_logconfig_handler, false);
     }
 
     if (start_immediately) {

src/lib/config/ccsession.h

@@ -354,6 +354,25 @@ private:
     ModuleSpec fetchRemoteSpec(const std::string& module, bool is_filename);
 };
 
+/// \brief Default handler for logging config updates
+///
+/// When CCSession is initialized with handle_logging set to true,
+/// this callback will be used to update the logger when a configuration
+/// change comes in.
+///
+/// This function updates the (global) loggers by initializing a
+/// LoggerManager and passing the settings as specified in the given
+/// configuration update.
+///
+/// \param n The name of the module
+/// \param new_config The modified configuration values
+/// \param config_data The full config data for the (remote) logging
+///                    module.
+void
+default_logconfig_handler(const std::string&n,
+                          isc::data::ConstElementPtr new_config,
+                          const ConfigData& config_data);
+
 }
 }
 #endif // __CCSESSION_H

src/lib/python/isc/config/ccsession.py

@@ -41,6 +41,7 @@ from isc.config.config_data import ConfigData, MultiConfigData, BIND10_CONFIG_DA
 import isc
 from isc.util.file import path_search
 import bind10_config
+from isc.log import log_config_update
 
 class ModuleCCSessionError(Exception): pass
 
@@ -119,7 +120,15 @@ def create_command(command_name, params = None):
     return msg
 
 def default_logconfig_handler(new_config, config_data):
-    pass
+    errors = []
+
+    if config_data.get_module_spec().validate_config(False, new_config, errors):
+        isc.log.log_config_update(new_config, config_data.get_module_spec().get_full_spec())
+    else:
+        # no logging here yet, TODO: log these errors
+        print("Error in logging configuration, ignoring config update: ")
+        for err in errors:
+            print(err)
 
 class ModuleCCSession(ConfigData):
     """This class maintains a connection to the command channel, as

src/lib/python/isc/config/tests/ccsession_test.py

@@ -23,6 +23,9 @@ from isc.config.ccsession import *
 from isc.config.config_data import BIND10_CONFIG_DATA_VERSION
 from unittest_fakesession import FakeModuleCCSession, WouldBlockForever
 
+# Is this test ever called outside of this directory?
+LOGGING_SPEC_FILE = "../../../../../bin/cfgmgr/plugins/logging.spec"
+
 class TestHelperFunctions(unittest.TestCase):
     def test_parse_answer(self):
         self.assertRaises(ModuleCCSessionError, parse_answer, 1)
@@ -604,7 +607,44 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
         self.assertEqual(len(fake_session.message_queue), 0)
-        
+
+    def test_logconfig_handler(self):
+        # test whether default_logconfig_handler reacts nicely to
+        # bad data. We assume the actual logger output is tested
+        # elsewhere
+        #print(config_data.get_value("loggers"))
+        self.assertRaises(TypeError, default_logconfig_handler);
+        self.assertRaises(TypeError, default_logconfig_handler, 1);
+
+        spec = isc.config.module_spec_from_file(LOGGING_SPEC_FILE,
+                                                bind10_config.PLUGIN_PATHS)
+        config_data = ConfigData(spec)
+
+        self.assertRaises(TypeError, default_logconfig_handler, 1, config_data)
+
+        default_logconfig_handler({}, config_data)
+
+        # Wrong data should not raise, but simply not be accepted
+        # This would log a lot of errors, so we may want to suppress that later
+        default_logconfig_handler({ "bad_data": "indeed" }, config_data)
+        default_logconfig_handler({ "bad_data": 1}, config_data)
+        default_logconfig_handler({ "bad_data": 1123 }, config_data)
+        default_logconfig_handler({ "bad_data": True }, config_data)
+        default_logconfig_handler({ "bad_data": False }, config_data)
+        default_logconfig_handler({ "bad_data": 1.1 }, config_data)
+        default_logconfig_handler({ "bad_data": [] }, config_data)
+        default_logconfig_handler({ "bad_data": [[],[],[[1, 3, False, "foo" ]]] },
+                                  config_data)
+        default_logconfig_handler({ "bad_data": [ 1, 2, { "b": { "c": "d" } } ] },
+                                  config_data)
+
+        # Try a correct config
+        log_conf = {"loggers":
+                       [{"name": "b10-xfrout", "output_options":
+                           [{"output": "/tmp/bind10.log",
+                                       "destination": "file",
+                                       "flush": True}]}]}
+        default_logconfig_handler(log_conf, config_data)
 
 class fakeData:
     def decode(self):

src/lib/python/isc/log/Makefile.am

@@ -12,6 +12,9 @@ log_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 log_la_LDFLAGS = $(PYTHON_LDFLAGS)
 log_la_LDFLAGS += -module
 log_la_LIBADD = $(top_builddir)/src/lib/log/liblog.la
+log_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la
+log_la_LIBADD += $(top_builddir)/src/lib/config/libcfgclient.la
+log_la_LIBADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 log_la_LIBADD += $(PYTHON_LIB)
 
 # This is not installed, it helps locate the module during tests

src/lib/python/isc/log/log.cc

@@ -22,6 +22,8 @@
 #include <log/logger_manager.h>
 #include <log/logger.h>
 
+#include <config/ccsession.h>
+
 #include <string>
 #include <boost/bind.hpp>
 
@@ -35,6 +37,9 @@ namespace {
 // NULL and will use the global dictionary.
 MessageDictionary* testDictionary = NULL;
 
+// To propagate python exceptions trough our code
+class InternalError {};
+
 PyObject*
 setTestDictionary(PyObject*, PyObject* args) {
     PyObject* enableO;
@@ -163,6 +168,73 @@ init(PyObject*, PyObject* args) {
     Py_RETURN_NONE;
 }
 
+isc::data::ElementPtr PyObjectToElement(PyObject* obj) {
+    if (PyBool_Check(obj)) {
+        return isc::data::Element::create(PyObject_IsTrue(obj) == 1);
+    } else if (PyLong_Check(obj)) {
+        return isc::data::Element::create(PyLong_AsLong(obj));
+    } else if (PyFloat_Check(obj)) {
+        return isc::data::Element::create(PyFloat_AsDouble(obj));
+    } else if (PyUnicode_Check(obj)) {
+        return isc::data::Element::create(PyBytes_AsString(PyUnicode_AsUTF8String(obj)));
+    } else if (PyList_Check(obj)) {
+        isc::data::ElementPtr result = isc::data::Element::createList();
+        for (Py_ssize_t i = 0; i < PyList_Size(obj); ++i) {
+            result->add(PyObjectToElement(PyList_GetItem(obj, i)));
+        }
+        return result;
+    } else if (PyDict_Check(obj)) {
+        isc::data::ElementPtr result = isc::data::Element::createMap();
+        PyObject *key, *value;
+        Py_ssize_t pos = 0;
+        while (PyDict_Next(obj, &pos, &key, &value)) {
+            result->set(PyBytes_AsString(PyUnicode_AsUTF8String(key)),
+                        PyObjectToElement(value));
+        }
+        return result;
+    } else if (obj == Py_None) {
+        return isc::data::ElementPtr();
+    } else {
+        throw InternalError();
+        return isc::data::ElementPtr();
+    }
+}
+
+PyObject*
+logConfigUpdate(PyObject*, PyObject* args) {
+    // we have no wrappers for ElementPtr and ConfigData,
+    // So we convert them on the spot.
+    // The new_config object is assumed to have been validated.
+    // If we need this code in other places, we should move it out,
+    // or consider full wrappers
+    PyObject* new_configO;
+    PyObject* mod_specO;
+    if (!PyArg_ParseTuple(args, "OO", &new_configO, &mod_specO)) {
+        return (NULL);
+    }
+
+    try {
+        isc::data::ElementPtr new_config = PyObjectToElement(new_configO);
+        isc::data::ElementPtr mod_spec_e = PyObjectToElement(mod_specO);
+        isc::config::ModuleSpec mod_spec(mod_spec_e);
+        isc::config::ConfigData config_data(mod_spec);
+        isc::config::default_logconfig_handler("logging", new_config,
+                                               config_data);
+
+        Py_RETURN_NONE;
+    } catch (const InternalError& ie) {
+        PyErr_SetString(PyExc_TypeError, "argument passed to log_config_update "
+                                         "is not a (compound) basic type");
+        return (NULL);
+    } catch (const std::exception& e) {
+        PyErr_SetString(PyExc_RuntimeError, e.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(PyExc_RuntimeError, "Unknown C++ exception");
+        return (NULL);
+    }
+}
+
 PyMethodDef methods[] = {
     {"set_test_dictionary", setTestDictionary, METH_VARARGS,
         "Set or unset testing mode for message dictionary. In testing, "
@@ -184,6 +256,18 @@ PyMethodDef methods[] = {
         "logging severity (one of 'DEBUG', 'INFO', 'WARN', 'ERROR' or "
         "'FATAL'), a debug level (integer in the range 0-99) and a file name "
         "of a dictionary with message text translations."},
+    {"log_config_update", logConfigUpdate, METH_VARARGS,
+        "Update logger settings. This method is automatically used when "
+        "ModuleCCSession is initialized with handle_logging_config set "
+        "to True. When called, the first argument is the new logging "
+        "configuration (as a basic data type). The second argument is "
+        "the raw specification (as returned from "
+        "ConfigData.get_module_spec().get_full_spec()."
+        "Raises a TypeError if either argument is not a basic type, or "
+        "if it contains a list or dict that does not contain a basic "
+        "type. If this call succeeds, the global logger settings have "
+        "been updated."
+    },
     {NULL, NULL, 0, NULL}
 };
 
@@ -361,9 +445,6 @@ Logger_isDebugEnabled(LoggerWrapper* self, PyObject* args) {
     }
 }
 
-// To propagate python exceptions trough our code
-class InternalError {};
-
 string
 objectToStr(PyObject* object, bool convert) {
     PyObject* cleanup(NULL);