BIND 10 #181: python wrappers to support xfr

[BIND 10 Development]

#181: python wrappers to support xfr
----------------------+-----------------------------------------------------
 Reporter:  larissas  |        Owner:  jelte                                         
     Type:  task      |       Status:  reviewing                                     
 Priority:  major     |    Milestone:  05. 3rd Incremental Release: Serious Secondary
Component:  xfrin     |   Resolution:                                                
 Keywords:            |    Sensitive:  0                                             
----------------------+-----------------------------------------------------
Changes (by stephen):

  * owner:  stephen => jelte


Comment:

 Just a few minor things missed (listed below), but otherwise all done;
 thank you.  I have a couple of remarks that may be relevant either as a
 further ticket or for general discussion so I will mention them here so
 they are recorded somewhere.


 __experiments/python-binding/src/lib/dns/python/libdns_python_common.cc__
 Remark: the associated header file is included using angle brackets, not
 quotes.  Strictly speaking the only difference between the two is that the
 '#include <file>' form searches for the file in the standard list of
 system directories, whereas the '#include "file"' form causes the
 directory holding the file being compiled to be searched as well.
 However, as the list of include directories is extended by the "-I" switch
 on the command line, either form works.  My preference - and it is just a
 personal one - is to use the '<>' form to indicate that the file is an
 operating system-supplied one and the '""' form for everything else.


 __experiments/python-binding/src/lib/dns/python/name_python.cc__
 >> Name_concatenate: I'm a bit uncomfortable that the catch is only for
 one of the
 >> possible exceptions that can be thrown by the "Name" constructor.
 (Although I agree
 >> that this is the only exception that can be logically thrown; however
 it does rely
 >> on the internals of the constructor itself. Perhaps a comment to that
 effect?)
 >
 > Umz, i don't think the copy constructor throws anything, right (well
 except bad_alloc again).
 > So only !TooLongName? can be thrown here, or am I missing something?

 Remark: No you are not - I'm just being overly cautious.  My point was
 that the constructor can throw a number of exceptions but only one is
 caught here.  We are relying on a knowledge of constructor internals and
 on the knowledge of the arguments passed to it to justify not catching the
 others. A change of implementation could lead to other exceptions being
 thrown in the future.  So future-proofing the code would involve:

 {{{
    try {
       ...
    } catch (isc::dns::TooLongName tln) {
       ...
    } catch (isc::Exception ise) {
       // Any other programmer-defined exception; return an unexpected
 exception message
    }
 }}}

 But this is a general point regarding defensive programming that should be
 discussed elsewhere.


 __experiments/python-binding/src/lib/dns/python/name_python.cc__
 > initModulePart_Name: When adding the constants to the module, can't the
 > numeric values in the calls to Py_BuildValue be accessed symbolically
 via
 > a call to Name::<constant_name>?

 This has been done for the !NameComparisonResult constants, but not for
 the constants such as "MAX_WIRE" in the Py_BuildValue calls embedded in
 calls to addClassVariable.


 __experiments/python-binding/src/lib/dns/python/rrttl.cc__
 There is a C-style cast in RRTTL_toWire().


 __experiments/python-
 binding/src/lib/dns/python/tests/message_python_test.py__
 Still a direct call to the {{{__str__()}}} method (instead of using the
 str() function).


 __experiments/python-
 binding/src/lib/dns/python/tests/rrtype_python_test.py__
 Still a direct call to the {{{__str__()}}} method (instead of using the
 str() function).

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