BIND 10 trac384, updated. ef67acec41e9b83d4aacff8de12e6a3e37628227 [trac384] fix the two 'types' of default

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jan 31 23:31:46 UTC 2011


The branch, trac384 has been updated
       via  ef67acec41e9b83d4aacff8de12e6a3e37628227 (commit)
       via  184c6c7068ec90f2a85a859bfa56d779a4294382 (commit)
       via  7c7c776676860ba64571154faef440093799e996 (commit)
      from  db105a36beb8999996f9e4853a2cceadb074775f (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 ef67acec41e9b83d4aacff8de12e6a3e37628227
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Feb 1 00:29:00 2011 +0100

    [trac384] fix the two 'types' of default
    
    When the default for an element is asked, and the element is one in a
    list, there are two options; a list can have a default value (like an
    empty list), but its elements can have their own default values. This
    change makes it so that if the current value of a list is its default,
    it'll find the element in there. If the list is *not* the default, it'll
    return the default for the element itself (i.e. an element has been
    added to a list, but not all of the elements values have been set)

commit 184c6c7068ec90f2a85a859bfa56d779a4294382
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Jan 31 16:38:31 2011 +0100

    [trac384] more review comments addressed

commit 7c7c776676860ba64571154faef440093799e996
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Jan 31 16:03:25 2011 +0100

    [trac384] handle review comments
    
    actually just the first part of it

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

Summary of changes:
 src/bin/bindctl/bindcmd.py               |  209 +++++++++++++++---------------
 src/bin/bindctl/moduleinfo.py            |   40 ++++---
 src/bin/bindctl/tests/bindctl_test.py    |   80 +++++++++++-
 src/lib/python/isc/config/ccsession.py   |    3 +-
 src/lib/python/isc/config/config_data.py |   50 +++++---
 5 files changed, 241 insertions(+), 141 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/bindctl/bindcmd.py b/src/bin/bindctl/bindcmd.py
index e35e971..e606998 100644
--- a/src/bin/bindctl/bindcmd.py
+++ b/src/bin/bindctl/bindcmd.py
@@ -375,7 +375,15 @@ class BindCmdInterpreter(Cmd):
         if cmd.command == "help" or ("help" in cmd.params.keys()):
             self._handle_help(cmd)
         elif cmd.module == CONFIG_MODULE_NAME:
-            self.apply_config_cmd(cmd)
+            try:
+                self.apply_config_cmd(cmd)
+            except isc.cc.data.DataTypeError as dte:
+                print("Error: " + str(dte))
+            except isc.cc.data.DataNotFoundError as dnfe:
+                print("Error: " + str(dnfe))
+            except KeyError as ke:
+                print("Error: missing " + str(ke))
+                raise ke
         else:
             self.apply_cmd(cmd)
 
@@ -398,17 +406,22 @@ class BindCmdInterpreter(Cmd):
         print(CONST_BINDCTL_HELP)
         for k in self.modules.values():
             n = k.get_name()
-            if len(n) >= 8:
-                print("\t%s" % n)
+            if len(n) >= CONST_BINDCTL_HELP_INDENT_WIDTH:
+                print("    %s" % n)
                 print(textwrap.fill(k.get_desc(),
-                      initial_indent="\t\t",
-                      subsequent_indent="\t\t",
+                      initial_indent="            ",
+                      subsequent_indent="    " +
+                      " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
                       width=70))
             else:
-                print(textwrap.fill("%s\t%s" % (k.get_name(), k.get_desc()),
-                      initial_indent="\t",
-                      subsequent_indent="\t\t",
-                      width=70))
+                print(textwrap.fill("%s%s%s" %
+                    (k.get_name(),
+                     " "*(CONST_BINDCTL_HELP_INDENT_WIDTH - len(k.get_name())),
+                     k.get_desc()),
+                    initial_indent="    ",
+                    subsequent_indent="    " +
+                    " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                    width=70))
     
     def onecmd(self, line):
         if line == 'EOF' or line.lower() == "quit":
@@ -551,102 +564,94 @@ class BindCmdInterpreter(Cmd):
            Raises a KeyError if the command was not complete
         '''
         identifier = self.location
-        try:
-            if 'identifier' in cmd.params:
-                if not identifier.endswith("/"):
-                    identifier += "/"
-                if cmd.params['identifier'].startswith("/"):
-                    identifier = cmd.params['identifier']
-                else:
-                    if cmd.params['identifier'].startswith('['):
-                        identifier = identifier[:-1]
-                    identifier += cmd.params['identifier']
-
-                # Check if the module is known; for unknown modules
-                # we currently deny setting preferences, as we have
-                # no way yet to determine if they are ok.
-                module_name = identifier.split('/')[1]
-                if module_name != "" and (self.config_data is None or \
-                   not self.config_data.have_specification(module_name)):
-                    print("Error: Module '" + module_name + "' unknown or not running")
-                    return
+        if 'identifier' in cmd.params:
+            if not identifier.endswith("/"):
+                identifier += "/"
+            if cmd.params['identifier'].startswith("/"):
+                identifier = cmd.params['identifier']
+            else:
+                if cmd.params['identifier'].startswith('['):
+                    identifier = identifier[:-1]
+                identifier += cmd.params['identifier']
+
+            # Check if the module is known; for unknown modules
+            # we currently deny setting preferences, as we have
+            # no way yet to determine if they are ok.
+            module_name = identifier.split('/')[1]
+            if module_name != "" and (self.config_data is None or \
+               not self.config_data.have_specification(module_name)):
+                print("Error: Module '" + module_name + "' unknown or not running")
+                return
 
-            if cmd.command == "show":
-                # check if we have the 'all' argument
-                show_all = False
-                if 'argument' in cmd.params:
-                    if cmd.params['argument'] == 'all':
-                        show_all = True
-                    elif 'identifier' not in cmd.params:
-                        # no 'all', no identifier, assume this is the
-                        #identifier
-                        identifier += cmd.params['argument']
-                    else:
-                        print("Error: unknown argument " + cmd.params['argument'] + ", or multiple identifiers given")
-                        return
-                values = self.config_data.get_value_maps(identifier, show_all)
-                for value_map in values:
-                    line = value_map['name']
-                    if value_map['type'] in [ 'module', 'map' ]:
-                        line += "/"
-                    elif len(value_map) > 1 and value_map['type'] == 'list' \
-                         and (value_map['value'] != []):
-                        # do not print content of non-empty lists if
-                        # we have more data to show
-                        line += "/"
-                    else:
-                        line += "\t" + json.dumps(value_map['value'])
-                    line += "\t" + value_map['type']
-                    line += "\t"
-                    if value_map['default']:
-                        line += "(default)"
-                    if value_map['modified']:
-                        line += "(modified)"
-                    print(line)
-            elif cmd.command == "show_json":
-                if identifier == "":
-                    print("Need at least the module to show the configuration in JSON format")
+        if cmd.command == "show":
+            # check if we have the 'all' argument
+            show_all = False
+            if 'argument' in cmd.params:
+                if cmd.params['argument'] == 'all':
+                    show_all = True
+                elif 'identifier' not in cmd.params:
+                    # no 'all', no identifier, assume this is the
+                    #identifier
+                    identifier += cmd.params['argument']
                 else:
-                    data, default = self.config_data.get_value(identifier)
-                    print(json.dumps(data))
-            elif cmd.command == "add":
-                if 'value' in cmd.params:
-                    self.config_data.add_value(identifier, cmd.params['value'])
-                else:
-                    self.config_data.add_value(identifier)
-            elif cmd.command == "remove":
-                if 'value' in cmd.params:
-                    self.config_data.remove_value(identifier, cmd.params['value'])
-                else:
-                    self.config_data.remove_value(identifier, None)
-            elif cmd.command == "set":
-                if 'identifier' not in cmd.params:
-                    print("Error: missing identifier or value")
+                    print("Error: unknown argument " + cmd.params['argument'] + ", or multiple identifiers given")
+                    return
+            values = self.config_data.get_value_maps(identifier, show_all)
+            for value_map in values:
+                line = value_map['name']
+                if value_map['type'] in [ 'module', 'map' ]:
+                    line += "/"
+                elif value_map['type'] == 'list' \
+                     and value_map['value'] != []:
+                    # do not print content of non-empty lists if
+                    # we have more data to show
+                    line += "/"
                 else:
-                    parsed_value = None
-                    try:
-                        parsed_value = json.loads(cmd.params['value'])
-                    except Exception as exc:
-                        # ok could be an unquoted string, interpret as such
-                        parsed_value = cmd.params['value']
-                    self.config_data.set_value(identifier, parsed_value)
-            elif cmd.command == "unset":
-                self.config_data.unset(identifier)
-            elif cmd.command == "revert":
-                self.config_data.clear_local_changes()
-            elif cmd.command == "commit":
-                self.config_data.commit()
-            elif cmd.command == "diff":
-                print(self.config_data.get_local_changes());
-            elif cmd.command == "go":
-                self.go(identifier)
-        except isc.cc.data.DataTypeError as dte:
-            print("Error: " + str(dte))
-        except isc.cc.data.DataNotFoundError as dnfe:
-            print("Error: " + str(dnfe))
-        except KeyError as ke:
-            print("Error: missing " + str(ke))
-            raise ke
+                    line += "\t" + json.dumps(value_map['value'])
+                line += "\t" + value_map['type']
+                line += "\t"
+                if value_map['default']:
+                    line += "(default)"
+                if value_map['modified']:
+                    line += "(modified)"
+                print(line)
+        elif cmd.command == "show_json":
+            if identifier == "":
+                print("Need at least the module to show the configuration in JSON format")
+            else:
+                data, default = self.config_data.get_value(identifier)
+                print(json.dumps(data))
+        elif cmd.command == "add":
+            if 'value' in cmd.params:
+                self.config_data.add_value(identifier, cmd.params['value'])
+            else:
+                self.config_data.add_value(identifier)
+        elif cmd.command == "remove":
+            if 'value' in cmd.params:
+                self.config_data.remove_value(identifier, cmd.params['value'])
+            else:
+                self.config_data.remove_value(identifier, None)
+        elif cmd.command == "set":
+            if 'identifier' not in cmd.params:
+                print("Error: missing identifier or value")
+            else:
+                parsed_value = None
+                try:
+                    parsed_value = json.loads(cmd.params['value'])
+                except Exception as exc:
+                    # ok could be an unquoted string, interpret as such
+                    parsed_value = cmd.params['value']
+                self.config_data.set_value(identifier, parsed_value)
+        elif cmd.command == "unset":
+            self.config_data.unset(identifier)
+        elif cmd.command == "revert":
+            self.config_data.clear_local_changes()
+        elif cmd.command == "commit":
+            self.config_data.commit()
+        elif cmd.command == "diff":
+            print(self.config_data.get_local_changes());
+        elif cmd.command == "go":
+            self.go(identifier)
 
     def go(self, identifier):
         '''Handles the config go command, change the 'current' location
diff --git a/src/bin/bindctl/moduleinfo.py b/src/bin/bindctl/moduleinfo.py
index 7d2e596..2f8addb 100644
--- a/src/bin/bindctl/moduleinfo.py
+++ b/src/bin/bindctl/moduleinfo.py
@@ -32,6 +32,9 @@ MODULE_NODE_NAME = 'module'
 COMMAND_NODE_NAME = 'command'
 PARAM_NODE_NAME = 'param'
 
+# this is used to align the descriptions in help output
+CONST_BINDCTL_HELP_INDENT_WIDTH=12
+
 
 class ParamInfo:
     """One parameter of one command.
@@ -164,11 +167,11 @@ class CommandInfo:
         mandatory_infos = []
         for info in params.values():            
             if not info.is_optional:
-                print("\t%s" % info.get_name())
+                print("    %s" % info.get_name())
                 print(textwrap.fill(info.get_desc(),
-                      initial_indent="\t\t",
-                      subsequent_indent="\t\t",
-                      width=50))
+                      initial_indent="        ",
+                      subsequent_indent="        ",
+                      width=70))
                 mandatory_infos.append(info)
 
         optional_infos = [info for info in params.values() 
@@ -176,11 +179,11 @@ class CommandInfo:
         if len(optional_infos) > 0:
             print("\nOptional parameters:")      
             for info in optional_infos:
-                print("\t%s" % info.get_name())
+                print("    %s" % info.get_name())
                 print(textwrap.fill(info.get_desc(),
-                      initial_indent="\t\t",
-                      subsequent_indent="\t\t",
-                      width=50))
+                      initial_indent="        ",
+                      subsequent_indent="        ",
+                      width=70))
 
 
 class ModuleInfo:
@@ -230,17 +233,22 @@ class ModuleInfo:
         print("Module ", self, "\nAvailable commands:")
         for k in self.commands.values():
             n = k.get_name()
-            if len(n) >= 8:
-                print("\t%s" % n)
+            if len(n) >= CONST_BINDCTL_HELP_INDENT_WIDTH:
+                print("    %s" % n)
                 print(textwrap.fill(k.get_desc(),
-                      initial_indent="\t\t",
-                      subsequent_indent="\t\t",
+                      initial_indent="            ",
+                      subsequent_indent="    " +
+                      " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
                       width=70))
             else:
-                print(textwrap.fill("%s\t%s" % (k.get_name(), k.get_desc()),
-                      initial_indent="\t",
-                      subsequent_indent="\t\t",
-                      width=70))
+                print(textwrap.fill("%s%s%s" %
+                    (k.get_name(),
+                     " "*(CONST_BINDCTL_HELP_INDENT_WIDTH - len(k.get_name())),
+                     k.get_desc()),
+                    initial_indent="    ",
+                    subsequent_indent="    " +
+                    " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                    width=70))
             
     def command_help(self, command):
         """Prints the help info for the command with the given name.
diff --git a/src/bin/bindctl/tests/bindctl_test.py b/src/bin/bindctl/tests/bindctl_test.py
index 653c908..aa3d52c 100644
--- a/src/bin/bindctl/tests/bindctl_test.py
+++ b/src/bin/bindctl/tests/bindctl_test.py
@@ -17,6 +17,8 @@
 import unittest
 import isc.cc.data
 import os
+from isc.config.config_data import ConfigData, MultiConfigData
+from isc.config.module_spec import ModuleSpec
 from bindctl import cmdparse
 from bindctl import bindcmd
 from bindctl.moduleinfo import *
@@ -238,11 +240,85 @@ class TestNameSequence(unittest.TestCase):
             assert self.random_names[i] == module_names[i+1]
             i = i + 1
 
-    def test_apply_cfg_command(self):
+# tine class to fake a UIModuleCCSession, but only the config data
+# parts for the next set of tests
+class FakeCCSession(MultiConfigData):
+    def __init__(self):
+        self._local_changes = {}
+        self._current_config = {}
+        self._specifications = {}
+        self.add_foo_spec()
+
+    def add_foo_spec(self):
+        spec = { "module_name": "foo",
+                 "config_data": [
+                 { "item_name": "an_int",
+                   "item_type": "integer",
+                   "item_optional": False,
+                   "item_default": 1
+                 },
+                 { "item_name": "a_list",
+                   "item_type": "list",
+                   "item_optional": False,
+                   "item_default": [],
+                   "list_item_spec":
+                   { "item_name": "a_string",
+                     "item_type": "string",
+                     "item_optional": False,
+                     "item_default": "bar"
+                   }
+                 }
+                 ]
+               }
+        self.set_specification(ModuleSpec(spec))
+    
+
+class TestConfigCommands(unittest.TestCase):
+    def setUp(self):
+        self.tool = bindcmd.BindCmdInterpreter()
+        mod_info = ModuleInfo(name = "foo")
+        self.tool.add_module_info(mod_info)
+        self.tool.config_data = FakeCCSession()
+        
+    def test_apply_cfg_command_int(self):
         self.tool.location = '/'
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/bar\" value=\"5\"")
+
+        self.assertEqual((1, MultiConfigData.DEFAULT),
+                         self.tool.config_data.get_value("/foo/an_int"))
+
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/an_int\" value=\"5\"")
         self.tool.apply_config_cmd(cmd)
+        self.assertEqual((5, MultiConfigData.LOCAL),
+                         self.tool.config_data.get_value("/foo/an_int"))
+
+        # this should raise a NotFoundError
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/bar\" value=\"[]\"")
+        self.assertRaises(isc.cc.data.DataNotFoundError, self.tool.apply_config_cmd, cmd)
+
+        # this should raise a TypeError
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/an_int\" value=\"[]\"")
+        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+
+    def test_apply_cfg_command_list(self):
+        self.tool.location = '/'
+
+        self.assertEqual(([], MultiConfigData.DEFAULT),
+                         self.tool.config_data.get_value("/foo/a_list"))
+
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=[\"a\"]")
+        self.tool.apply_config_cmd(cmd)
+        self.assertEqual((["a"], MultiConfigData.LOCAL),
+                         self.tool.config_data.get_value("/foo/a_list"))
+
+        # this should raise a TypeError
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=\"a\"")
+        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+        
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=[1]")
+        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+
     
+
 class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
     def __init__(self):
         pass
diff --git a/src/lib/python/isc/config/ccsession.py b/src/lib/python/isc/config/ccsession.py
index eb5623b..9d2682e 100644
--- a/src/lib/python/isc/config/ccsession.py
+++ b/src/lib/python/isc/config/ccsession.py
@@ -401,8 +401,7 @@ class UIModuleCCSession(MultiConfigData):
             
         if value not in cur_list:
             cur_list.append(value)
-
-        self.set_value(identifier, cur_list)
+            self.set_value(identifier, cur_list)
 
     def remove_value(self, identifier, value_str):
         """Remove a value from a configuration list. The value string
diff --git a/src/lib/python/isc/config/config_data.py b/src/lib/python/isc/config/config_data.py
index 3fc9b54..b2d71c4 100644
--- a/src/lib/python/isc/config/config_data.py
+++ b/src/lib/python/isc/config/config_data.py
@@ -129,7 +129,7 @@ def find_spec_part(element, identifier):
                     cur_el = cur_el_item
                     found = True
             if not found:
-                raise isc.cc.data.DataNotFoundError(id + " in " + str(cur_el))
+                raise isc.cc.data.DataNotFoundError(id + " not found")
         elif type(cur_el) == dict and 'list_item_spec' in cur_el.keys():
             cur_el = cur_el['list_item_spec']
         elif type(cur_el) == list:
@@ -146,7 +146,7 @@ def find_spec_part(element, identifier):
                             cur_el = cur_el["list_item_spec"]
                     found = True
             if not found:
-                raise isc.cc.data.DataNotFoundError(id + " in " + str(cur_el))
+                raise isc.cc.data.DataNotFoundError(id + " not found")
         else:
             raise isc.cc.data.DataNotFoundError("Not a correct config specification")
     return cur_el
@@ -378,24 +378,34 @@ class MultiConfigData:
             while len(id_parts) > 0:
                 id_part = id_parts.pop(0)
                 item_id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+                id_list = module + "/" + id_prefix + "/" + item_id
                 id_prefix += "/" + id_part
                 if list_indices is not None:
-                    spec = find_spec_part(self._specifications[module].get_config_spec(), id_prefix)
-                    if 'item_default' in spec:
-                        list_value = spec['item_default']
-                        for i in list_indices:
-                            if i < len(list_value):
-                                list_value = list_value[i]
+                    # there's actually two kinds of default here for
+                    # lists; they can have a default value (like an
+                    # empty list), but their elements can  also have
+                    # default values.
+                    # So if the list item *itself* is a default,
+                    # we need to get the value out of that. If not, we
+                    # need to find the default for the specific element.
+                    list_value, type = self.get_value(id_list) 
+                    list_spec = find_spec_part(self._specifications[module].get_config_spec(), id_prefix)
+                    if type == self.DEFAULT:
+                        if 'item_default' in list_spec:
+                            list_value = list_spec['item_default']
+                            for i in list_indices:
+                                if i < len(list_value):
+                                    list_value = list_value[i]
+                                else:
+                                    # out of range, return None
+                                    return None
+                                
+                            if len(id_parts) > 0:
+                                rest_of_id = "/".join(id_parts)
+                                return isc.cc.data.find(list_value, rest_of_id)
                             else:
-                                # out of range, return None
-                                return None
-                            
-                        if len(id_parts) > 0:
-                            rest_of_id = "/".join(id_parts)
-                            return isc.cc.data.find(list_value, rest_of_id)
-                        else:
-                            return list_value
-    
+                                return list_value
+                    
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
                 return spec['item_default']
@@ -476,8 +486,10 @@ class MultiConfigData:
            name: name of the entry (string)
            type: string containing the type of the value (or 'module')
            value: value of the entry if it is a string, int, double or bool
-           modified: true if the value is a local change
-           default: true if the value has been changed
+           modified: true if the value is a local change that has not
+                     been committed
+           default: true if the value has not been changed (i.e. the
+                    value is the default from the specification)
            TODO: use the consts for those last ones
            Throws DataNotFoundError if the identifier is bad
         """




More information about the bind10-changes mailing list