BIND 10 #405: basic support for addressing individual list items in configuration

BIND 10 Development do-not-reply at isc.org
Sun Dec 5 22:11:57 UTC 2010


#405: basic support for addressing individual list items in configuration
--------------------------------+-------------------------------------------
      Reporter:  jelte          |        Owner:  stephen  
          Type:  defect         |       Status:  reviewing
      Priority:  major          |    Milestone:           
     Component:  configuration  |   Resolution:           
      Keywords:                 |    Sensitive:  0        
Estimatedhours:  0.0            |        Hours:  0        
      Billable:  1              |   Totalhours:  0        
      Internal:  0              |  
--------------------------------+-------------------------------------------
Changes (by jelte):

  * owner:  jelte => stephen


Comment:

 Replying to [comment:3 stephen]:
 > '''src/lib/python/isc/cc/data.py'''[[BR]]
 > In find():
 > {{{
 >     if (type(element) != dict and identifier != ""):
 >         raise DataTypeError("element in find() is not a dict")
 > }}}
 > The error message will not be output if identifier is the empty string.
 >

 that was intentional, I updated the docstring, and refactored the code a
 little to make behaviour more clear, r3722

 > '''src/lib/python/isc/config/ccsession.py'''
 > In remove_value(), would like to see some comment as to why:[[BR]]
 > {{{
 >         if identifier == "":
 >             identifier = value_str
 >             value_str = None
 > }}}
 > ... is necessary as it means that xxx.remove_value("", "xyz") is the
 same as xxx.remove_value("xyz", None).  Why the two forms of the call?
 >

 Ohyeah that was a workaround for the way command arguments were parsed
 (identifier was optional, value used to be mandatory, so if only one
 argument was given, the system assumed it was the value, while for this
 case it should have been the identifier). If have a better solution; now
 both values are optional, so it does take the identifier if only one is
 given, and the reversal isn't necessary anymore.

 Please see r3723

 > '''src/lib/python/isc/config/tests/ccsession_test.py'''[[br]]
 > In test_add_remove_value:
 > {{{
 >         uccs.remove_value("Spec2/item5[0]", None)
 >         self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 >         uccs.remove_value("", "Spec2/item5[0]")
 >         self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 > }}}
 > The second remove_value() test does not check that it has the same
 effect as the first one, it only checks that it has no effect on the data
 - is that what is required?

 It was mostly to check if it didn't fail, but good catch, that wasn't
 supposed to test what it tested. However, with the change above, this test
 isn't necessary anymore (and indeed would have failed). It's also removed
 in r3723.

-- 
Ticket URL: <http://bind10.isc.org/ticket/405#comment:4>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list