From: Jelte Jansen Date: Wed, 8 Feb 2012 16:18:14 +0000 (+0100) Subject: [640] more comments X-Git-Tag: trac2351_base~261^2~1^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=61203697e8fc714835ef504a7e3eda925dfcda42;p=thirdparty%2Fkea.git [640] more comments --- diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc index 63fa4cdcf9..aa34c02a6a 100644 --- a/src/lib/config/ccsession.cc +++ b/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... } }; diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h index d3225d2cb6..059968c71a 100644 --- a/src/lib/config/ccsession.h +++ b/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. /// diff --git a/src/lib/python/isc/config/cfgmgr.py b/src/lib/python/isc/config/cfgmgr.py index f7ecce9bd4..dd9782725a 100644 --- a/src/lib/python/isc/config/cfgmgr.py +++ b/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': }. 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: diff --git a/src/lib/python/isc/config/tests/cfgmgr_test.py b/src/lib/python/isc/config/tests/cfgmgr_test.py index 2b7620aaea..0eb0171fc7 100644 --- a/src/lib/python/isc/config/tests/cfgmgr_test.py +++ b/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 ] diff --git a/tests/lettuce/features/terrain/bind10_control.py b/tests/lettuce/features/terrain/bind10_control.py index 921073842d..7ccd2b3b03 100644 --- a/tests/lettuce/features/terrain/bind10_control.py +++ b/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.