BIND 10 trac1010, updated. 4fae538655882db7c085dab798b4fb29c4a9d8f1 [trac1010] pass values as JSON strings instead of wrap-time conversion

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jun 20 13:06:46 UTC 2011


The branch, trac1010 has been updated
       via  4fae538655882db7c085dab798b4fb29c4a9d8f1 (commit)
       via  b5cfd5e541d4bbb7f13ad93392018711e19ba0e5 (commit)
      from  a4f1f8de765810aecff1194c74a108682e3de28e (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 4fae538655882db7c085dab798b4fb29c4a9d8f1
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Jun 20 14:47:38 2011 +0200

    [trac1010] pass values as JSON strings instead of wrap-time conversion

commit b5cfd5e541d4bbb7f13ad93392018711e19ba0e5
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Jun 20 12:34:46 2011 +0200

    [trac1010] address review comments

-----------------------------------------------------------------------

Summary of changes:
 src/lib/config/ccsession.cc              |    2 +-
 src/lib/config/ccsession.h               |    4 +-
 src/lib/python/isc/config/ccsession.py   |    4 +-
 src/lib/python/isc/log/log.cc            |   82 +++++++++---------------------
 src/lib/python/isc/log/tests/log_test.py |   17 ++++---
 5 files changed, 40 insertions(+), 69 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index f704598..dd2be3d 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -247,7 +247,7 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
 } // end anonymous namespace
 
 void
-default_logconfig_handler(const std::string&n,
+default_logconfig_handler(const std::string& module_name,
                           ConstElementPtr new_config,
                           const ConfigData& config_data) {
     config_data.getModuleSpec().validateConfig(new_config, true);
diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h
index 064c870..0d4b7f3 100644
--- a/src/lib/config/ccsession.h
+++ b/src/lib/config/ccsession.h
@@ -364,12 +364,12 @@ private:
 /// LoggerManager and passing the settings as specified in the given
 /// configuration update.
 ///
-/// \param n The name of the module
+/// \param module_name 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,
+default_logconfig_handler(const std::string& module_name,
                           isc::data::ConstElementPtr new_config,
                           const ConfigData& config_data);
 
diff --git a/src/lib/python/isc/config/ccsession.py b/src/lib/python/isc/config/ccsession.py
index 95e3a40..0869160 100644
--- a/src/lib/python/isc/config/ccsession.py
+++ b/src/lib/python/isc/config/ccsession.py
@@ -42,6 +42,7 @@ import isc
 from isc.util.file import path_search
 import bind10_config
 from isc.log import log_config_update
+import json
 
 class ModuleCCSessionError(Exception): pass
 
@@ -123,7 +124,8 @@ def default_logconfig_handler(new_config, config_data):
     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())
+        isc.log.log_config_update(json.dumps(new_config),
+            json.dumps(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: ")
diff --git a/src/lib/python/isc/log/log.cc b/src/lib/python/isc/log/log.cc
index 40eea67..c920e5b 100644
--- a/src/lib/python/isc/log/log.cc
+++ b/src/lib/python/isc/log/log.cc
@@ -38,6 +38,11 @@ namespace {
 MessageDictionary* testDictionary = NULL;
 
 // To propagate python exceptions trough our code
+// This exception is used to signal to the calling function that a
+// proper Python Exception has already been set, and the caller
+// should now return NULL.
+// Since it is only used internally, and should not pass any
+// information itself, is is not derived from std::exception
 class InternalError {};
 
 PyObject*
@@ -168,79 +173,39 @@ init(PyObject*, PyObject* args) {
     Py_RETURN_NONE;
 }
 
-// This function takes a PyObject* and converts it to a ConstElementPtr
-//
-// The Python object should be a basic object, i.e. a bool, long,
-// float, string, list, or dict. The contents of these lists and
-// dicts have the same limitation.
-// If any other type is encountered, an InternalError is raised.
-//
-// Note: This is a conversion between basic python types and our
-// own c++ equivalent (the Element). In that sense we may want to use it
-// in more places. If so, we should move it. It is defined here now
-// because this is the only place it is used.
-isc::data::ConstElementPtr 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();
-    }
-}
-
 PyObject*
 logConfigUpdate(PyObject*, PyObject* args) {
     // we have no wrappers for ElementPtr and ConfigData,
-    // So we convert them on the spot.
+    // So we expect JSON strings and convert them.
     // 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)) {
+
+    const char* new_config_json;
+    const char* mod_spec_json;
+    if (!PyArg_ParseTuple(args, "ss",
+                          &new_config_json, &mod_spec_json)) {
         return (NULL);
     }
 
     try {
-        isc::data::ConstElementPtr new_config = PyObjectToElement(new_configO);
-        isc::data::ConstElementPtr mod_spec_e = PyObjectToElement(mod_specO);
+        isc::data::ConstElementPtr new_config =
+            isc::data::Element::fromJSON(new_config_json);
+        isc::data::ConstElementPtr mod_spec_e =
+            isc::data::Element::fromJSON(mod_spec_json);
         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 isc::data::JSONError& je) {
+        std::string error_msg = std::string("JSON format error: ") + je.what();
+        PyErr_SetString(PyExc_TypeError, error_msg.c_str());
     } catch (const isc::data::TypeError& de) {
         PyErr_SetString(PyExc_TypeError, "argument 1 of log_config_update "
                                          "is not a map of config data");
     } catch (const isc::config::ModuleSpecError& mse) {
         PyErr_SetString(PyExc_TypeError, "argument 2 of log_config_update "
                                          "is not a correct module specification");
-    } catch (const InternalError& ie) {
-        PyErr_SetString(PyExc_TypeError, "argument passed to log_config_update "
-                                         "is not a (compound) basic type");
     } catch (const std::exception& e) {
         PyErr_SetString(PyExc_RuntimeError, e.what());
     } catch (...) {
@@ -274,12 +239,13 @@ PyMethodDef methods[] = {
         "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 "
+        "configuration (in JSON format). 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 "
+        "ConfigData.get_module_spec().get_full_spec(), and converted to "
+        "JSON format).\n"
+        "Raises a TypeError if either argument is not a (correct) JSON "
+        "string, or if the spec is not a correct spec.\n"
+        "If this call succeeds, the global logger settings have "
         "been updated."
     },
     {NULL, NULL, 0, NULL}
diff --git a/src/lib/python/isc/log/tests/log_test.py b/src/lib/python/isc/log/tests/log_test.py
index 4ee1fe3..af7e647 100644
--- a/src/lib/python/isc/log/tests/log_test.py
+++ b/src/lib/python/isc/log/tests/log_test.py
@@ -16,6 +16,7 @@
 # This tests it can be loaded, nothing more yet
 import isc.log
 import unittest
+import json
 import bind10_config
 from isc.config.ccsession import path_search
 
@@ -55,7 +56,7 @@ class Manager(unittest.TestCase):
         isc.log.init("root", "INFO", 0, "/no/such/file");
 
     def test_log_config_update(self):
-        log_spec = isc.config.module_spec_from_file(path_search('logging.spec', bind10_config.PLUGIN_PATHS)).get_full_spec()
+        log_spec = json.dumps(isc.config.module_spec_from_file(path_search('logging.spec', bind10_config.PLUGIN_PATHS)).get_full_spec())
 
         self.assertRaises(TypeError, isc.log.log_config_update)
         self.assertRaises(TypeError, isc.log.log_config_update, 1)
@@ -65,19 +66,21 @@ class Manager(unittest.TestCase):
         self.assertRaises(TypeError, isc.log.log_config_update, 1, log_spec)
         self.assertRaises(TypeError, isc.log.log_config_update, [], log_spec)
         self.assertRaises(TypeError, isc.log.log_config_update, "foo", log_spec)
+        self.assertRaises(TypeError, isc.log.log_config_update, "{ '", log_spec)
 
         # empty should pass
-        isc.log.log_config_update({}, log_spec)
+        #isc.log.log_config_update("{", log_spec)
+        isc.log.log_config_update("{}", log_spec)
 
         # bad spec
-        self.assertRaises(TypeError, isc.log.log_config_update, {}, {"foo": "bar"})
+        self.assertRaises(TypeError, isc.log.log_config_update, "{}", json.dumps({"foo": "bar"}))
 
         # Try a correct one
-        log_conf = {"loggers":
-                       [{"name": "b10-xfrout", "output_options":
-                           [{"output": "/tmp/bind10.log",
+        log_conf = json.dumps({"loggers":
+                                [{"name": "b10-xfrout", "output_options":
+                                    [{"output": "/tmp/bind10.log",
                                        "destination": "file",
-                                       "flush": True}]}]}
+                                       "flush": True}]}]})
         isc.log.log_config_update(log_conf, log_spec)
 
 class Logger(unittest.TestCase):




More information about the bind10-changes mailing list