[640] more comments

src/lib/config/ccsession.cc

@@ -490,14 +490,20 @@ ModuleCCSession::ModuleCCSession(
 }
 
 ModuleCCSession::~ModuleCCSession() {
+    // Logging itself can possibly throw too
+    // TODO: add exception-free logging calls
     try {
-        sendStopping();
-    } catch (const std::exception& exc) {
-        LOG_ERROR(config_logger,
-                  CONFIG_CCSESSION_STOPPING).arg(exc.what());
+        try {
+            sendStopping();
+        } catch (const std::exception& exc) {
+            LOG_ERROR(config_logger,
+                      CONFIG_CCSESSION_STOPPING).arg(exc.what());
+        } catch (...) {
+            LOG_ERROR(config_logger,
+                      CONFIG_CCSESSION_STOPPING_UNKNOWN);
+        }
     } catch (...) {
-        LOG_ERROR(config_logger,
-                  CONFIG_CCSESSION_STOPPING_UNKNOWN);
+        // Can't really do anything at this point...
     }
 };
 

src/lib/config/ccsession.h

@@ -195,10 +195,10 @@ public:
     ///
     /// Destructor
     ///
-    /// The desctructor automatically calls sendStopping(), which sends
+    /// The destructor automatically calls sendStopping(), which sends
     /// a message to the ConfigManager that this module is stopping
     ///
-    ~ModuleCCSession();
+    virtual ~ModuleCCSession();
 
     /// Start receiving new commands and configuration changes asynchronously.
     ///

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

@@ -297,7 +297,7 @@ class ConfigManager:
         """Write the current configuration to the file specificied at init()"""
         self.config.write_to_file()
 
-    def _handle_get_module_spec(self, cmd):
+    def __handle_get_module_spec(self, cmd):
         """Private function that handles the 'get_module_spec' command"""
         answer = {}
         if cmd != None:
@@ -318,7 +318,7 @@ class ConfigManager:
             answer = ccsession.create_answer(0, self.get_module_spec())
         return answer
 
-    def _handle_get_config_dict(self, cmd):
+    def __handle_get_config_dict(self, cmd):
         """Private function that handles the 'get_config' command
            where the command has been checked to be a dict"""
         if 'module_name' in cmd and cmd['module_name'] != '':
@@ -332,17 +332,17 @@ class ConfigManager:
         else:
             return ccsession.create_answer(1, "Bad module_name in get_config command")
 
-    def _handle_get_config(self, cmd):
+    def __handle_get_config(self, cmd):
         """Private function that handles the 'get_config' command"""
         if cmd != None:
             if type(cmd) == dict:
-                return self._handle_get_config_dict(cmd)
+                return self.__handle_get_config_dict(cmd)
             else:
                 return ccsession.create_answer(1, "Bad get_config command, argument not a dict")
         else:
             return ccsession.create_answer(0, self.config.data)
 
-    def _handle_set_config_module(self, module_name, cmd):
+    def __handle_set_config_module(self, module_name, cmd):
         # the answer comes (or does not come) from the relevant module
         # so we need a variable to see if we got it
         answer = None
@@ -405,7 +405,7 @@ class ConfigManager:
                 self.config.data = old_data
         return answer
 
-    def _handle_set_config_all(self, cmd):
+    def __handle_set_config_all(self, cmd):
         old_data = copy.deepcopy(self.config.data)
         got_error = False
         err_list = []
@@ -413,7 +413,7 @@ class ConfigManager:
         # sets, so we simply call set_config_module for each of those
         for module in cmd:
             if module != "version":
-                answer = self._handle_set_config_module(module, cmd[module])
+                answer = self.__handle_set_config_module(module, cmd[module])
                 if answer == None:
                     got_error = True
                     err_list.append("No answer message from " + module)
@@ -432,16 +432,16 @@ class ConfigManager:
             self.config.data = old_data
             return ccsession.create_answer(1, " ".join(err_list))
 
-    def _handle_set_config(self, cmd):
+    def __handle_set_config(self, cmd):
         """Private function that handles the 'set_config' command"""
         answer = None
 
         if cmd == None:
             return ccsession.create_answer(1, "Wrong number of arguments")
         if len(cmd) == 2:
-            answer = self._handle_set_config_module(cmd[0], cmd[1])
+            answer = self.__handle_set_config_module(cmd[0], cmd[1])
         elif len(cmd) == 1:
-            answer = self._handle_set_config_all(cmd[0])
+            answer = self.__handle_set_config_all(cmd[0])
         else:
             answer = ccsession.create_answer(1, "Wrong number of arguments")
         if not answer:
@@ -449,7 +449,7 @@ class ConfigManager:
 
         return answer
 
-    def _handle_module_spec(self, spec):
+    def __handle_module_spec(self, spec):
         """Private function that handles the 'module_spec' command"""
         # todo: validate? (no direct access to spec as
         # todo: use ModuleSpec class
@@ -460,7 +460,7 @@ class ConfigManager:
                                          spec.get_full_spec())
         return ccsession.create_answer(0)
 
-    def _handle_module_stopping(self, arg):
+    def __handle_module_stopping(self, arg):
         """Private function that handles a 'stopping' command;
            The argument is of the form { 'module_name': <name> }.
            If the module is known, it is removed from the known list,
@@ -495,19 +495,19 @@ class ConfigManager:
             elif cmd == ccsession.COMMAND_GET_STATISTICS_SPEC:
                 answer = ccsession.create_answer(0, self.get_statistics_spec())
             elif cmd == ccsession.COMMAND_GET_MODULE_SPEC:
-                answer = self._handle_get_module_spec(arg)
+                answer = self.__handle_get_module_spec(arg)
             elif cmd == ccsession.COMMAND_GET_CONFIG:
-                answer = self._handle_get_config(arg)
+                answer = self.__handle_get_config(arg)
             elif cmd == ccsession.COMMAND_SET_CONFIG:
-                answer = self._handle_set_config(arg)
+                answer = self.__handle_set_config(arg)
             elif cmd == ccsession.COMMAND_MODULE_STOPPING:
-                answer = self._handle_module_stopping(arg)
+                answer = self.__handle_module_stopping(arg)
             elif cmd == ccsession.COMMAND_SHUTDOWN:
                 self.running = False
                 answer = ccsession.create_answer(0)
             elif cmd == ccsession.COMMAND_MODULE_SPEC:
                 try:
-                    answer = self._handle_module_spec(isc.config.ModuleSpec(arg))
+                    answer = self.__handle_module_spec(isc.config.ModuleSpec(arg))
                 except isc.config.ModuleSpecError as dde:
                     answer = ccsession.create_answer(1, "Error in data definition: " + str(dde))
             else:

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

@@ -463,9 +463,9 @@ class TestConfigManager(unittest.TestCase):
             self.cm.set_virtual_module(self.spec, check_test)
             # The fake session will throw now if it tries to read a response.
             # Handy, we don't need to find a complicated way to check for it.
-            result = self.cm._handle_set_config_module(self.spec.
-                                                       get_module_name(),
-                                                       {'item1': value})
+            result = self.cm.handle_msg(ccsession.create_command(
+                        ccsession.COMMAND_SET_CONFIG,
+                        [self.spec.get_module_name(), { "item1": value }]))
             # Check the correct result is passed and our function was called
             # With correct data
             self.assertEqual(self.called_with['item1'], value)
@@ -497,19 +497,22 @@ class TestConfigManager(unittest.TestCase):
         self.assertEqual({"version": 2}, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value1": 123 }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value1": 123 }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value1": 123 }
                          }, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value1": 124 }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value1": 124 }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value1": 124 }
                          }, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value2": True }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value2": True }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value1": 124,
                                     "value2": True
@@ -517,7 +520,8 @@ class TestConfigManager(unittest.TestCase):
                          }, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value3": [ 1, 2, 3 ] }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value3": [ 1, 2, 3 ] }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value1": 124,
                                     "value2": True,
@@ -526,7 +530,8 @@ class TestConfigManager(unittest.TestCase):
                          }, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value2": False }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value2": False }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value1": 124,
                                     "value2": False,
@@ -535,7 +540,8 @@ class TestConfigManager(unittest.TestCase):
                          }, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value1": None }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value1": None }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value2": False,
                                     "value3": [ 1, 2, 3 ]
@@ -543,7 +549,8 @@ class TestConfigManager(unittest.TestCase):
                          }, self.cm.config.data)
 
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
-        self.cm._handle_set_config_all({"test": { "value3": [ 1 ] }})
+        self.cm.handle_msg(ccsession.create_command(
+            ccsession.COMMAND_SET_CONFIG, ["test", { "value3": [ 1 ] }]))
         self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
                           "test": { "value2": False,
                                     "value3": [ 1 ]

tests/lettuce/features/terrain/bind10_control.py

@@ -113,7 +113,7 @@ def have_bind10_running(step, config_file, cmdctl_port, process_name):
     step.given(wait_step)
 
 # function to send lines to bindctl, and store the result
-def run_bindctl(commands, cmdctl_port=47805):
+def run_bindctl(commands, cmdctl_port=None):
     """Run bindctl.
        Parameters:
        commands: a sequence of strings which will be sent.