BIND 10 trac640, updated. 373435e91924553439f4881a1fbf71aacd5fb4a1 [640] addressed new review comments

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Feb 9 15:30:02 UTC 2012


The branch, trac640 has been updated
       via  373435e91924553439f4881a1fbf71aacd5fb4a1 (commit)
      from  61203697e8fc714835ef504a7e3eda925dfcda42 (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 373435e91924553439f4881a1fbf71aacd5fb4a1
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Feb 9 16:29:49 2012 +0100

    [640] addressed new review comments

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

Summary of changes:
 src/bin/cmdctl/cmdctl.py.in                    |    2 +-
 src/bin/xfrout/xfrout.py.in                    |   19 ++++++++++++-------
 src/lib/config/ccsession.cc                    |   18 ++++++------------
 src/lib/python/isc/config/ccsession.py         |    4 ----
 src/lib/python/isc/config/tests/cfgmgr_test.py |   13 ++++---------
 5 files changed, 23 insertions(+), 33 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/cmdctl/cmdctl.py.in b/src/bin/cmdctl/cmdctl.py.in
index 5163c3a..74fc364 100755
--- a/src/bin/cmdctl/cmdctl.py.in
+++ b/src/bin/cmdctl/cmdctl.py.in
@@ -313,7 +313,7 @@ class CommandControl():
             # The 'value' of a specification update can be either
             # a specification, or None. In the first case, simply
             # set it. If it is None, delete the module if it is
-            # known (and ignore otherwise).
+            # known.
             with self._lock:
                 if args[1] is None:
                     if args[0] in self.modules_spec:
diff --git a/src/bin/xfrout/xfrout.py.in b/src/bin/xfrout/xfrout.py.in
index e23be76..5c82f19 100755
--- a/src/bin/xfrout/xfrout.py.in
+++ b/src/bin/xfrout/xfrout.py.in
@@ -974,13 +974,18 @@ class XfroutServer:
         self._notifier.shutdown()
         if self._unix_socket_server:
             self._unix_socket_server.shutdown()
-
-            # Wait for all threads to terminate
-            main_thread = threading.currentThread()
-            for th in threading.enumerate():
-                if th is main_thread:
-                    continue
-                th.join()
+        self._wait_for_threads()
+
+    def _wait_for_threads(self):
+        # Wait for all threads to terminate. this is a call that is only used
+        # in shutdown(), but it has its own method, so we can test shutdown
+        # without involving thread operations (the test would override this
+        # method)
+        main_thread = threading.currentThread()
+        for th in threading.enumerate():
+            if th is main_thread:
+                continue
+            th.join()
 
     def command_handler(self, cmd, args):
         if cmd == "shutdown":
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index aa34c02..63fa4cd 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -490,20 +490,14 @@ ModuleCCSession::ModuleCCSession(
 }
 
 ModuleCCSession::~ModuleCCSession() {
-    // Logging itself can possibly throw too
-    // TODO: add exception-free logging calls
     try {
-        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);
-        }
+        sendStopping();
+    } catch (const std::exception& exc) {
+        LOG_ERROR(config_logger,
+                  CONFIG_CCSESSION_STOPPING).arg(exc.what());
     } catch (...) {
-        // Can't really do anything at this point...
+        LOG_ERROR(config_logger,
+                  CONFIG_CCSESSION_STOPPING_UNKNOWN);
     }
 };
 
diff --git a/src/lib/python/isc/config/ccsession.py b/src/lib/python/isc/config/ccsession.py
index 5da56cf..b505399 100644
--- a/src/lib/python/isc/config/ccsession.py
+++ b/src/lib/python/isc/config/ccsession.py
@@ -438,9 +438,6 @@ class UIModuleCCSession(MultiConfigData):
             be run after each interaction (at this moment). It is usually
             also combined with request_current_config(). For that reason,
             we provide update_specs_and_config() which calls both."""
-        # this step should be unnecessary but is the current way cmdctl returns stuff
-        # so changes are needed there to make this clean (we need a command to simply get the
-        # full specs for everything, including commands etc, not separate gets for that)
         specs = self._conn.send_GET('/module_spec')
         self.clear_specifications()
         for module in specs.keys():
@@ -612,7 +609,6 @@ class UIModuleCCSession(MultiConfigData):
             # answer is either an empty dict (on success), or one
             # containing errors
             if answer == {}:
-                self.update_specs_and_config()
                 self.clear_local_changes()
             elif "error" in answer:
                 raise ModuleCCSessionError("Error: " + str(answer["error"]) + "\n" + "Configuration not committed")
diff --git a/src/lib/python/isc/config/tests/cfgmgr_test.py b/src/lib/python/isc/config/tests/cfgmgr_test.py
index 0eb0171..7fe8212 100644
--- a/src/lib/python/isc/config/tests/cfgmgr_test.py
+++ b/src/lib/python/isc/config/tests/cfgmgr_test.py
@@ -337,8 +337,10 @@ class TestConfigManager(unittest.TestCase):
         # Send the 'ok' that cfgmgr expects back to the fake queue first
         self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
 
+        config_version = config_data.BIND10_CONFIG_DATA_VERSION
         self._handle_msg_helper({ "command": [ "set_config",
-                                               [self.name, new_config] ] },
+                                               [ { "version": config_version,
+                                                 self.name: new_config } ] ] },
                                 my_ok_answer)
 
         # The cfgmgr should have eaten the ok message, and sent out an update
@@ -349,7 +351,7 @@ class TestConfigManager(unittest.TestCase):
 
         # Config should have been updated
         self.assertEqual(self.cm.config.data, {self.name: new_config,
-                            'version': config_data.BIND10_CONFIG_DATA_VERSION})
+                            'version': config_version})
 
         # and the queue should now be empty again
         self.assertEqual(len(self.fake_session.message_queue), 0)
@@ -439,13 +441,6 @@ class TestConfigManager(unittest.TestCase):
                                 None)
         self.assertEqual(len(self.fake_session.message_queue), 0)
 
-        # If the module does not exist, or is not seen as 'running', the
-        # same message should not cause new messages to be sent
-        self._handle_msg_helper({ "command": [ "stopping",
-                                               { "module_name": "Foo"}] },
-                                None)
-        self.assertEqual(len(self.fake_session.message_queue), 0)
-
     def test_set_config_virtual(self):
         """Test that if the module is virtual, we don't send it over the
            message bus, but call the checking function.




More information about the bind10-changes mailing list