BIND 10 #1484: python binding for ZoneFinder.findAll

[BIND 10 Development]

#1484: python binding for ZoneFinder.findAll
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  minor  |  Sprint-20120110
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:  DDNS   |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:11 jinmei]:
 > '''xfrin.py.in'''
 >
 > - not a problem of this branch, but the call to find() could be even
 >   simpler:
 > {{{#!python
 >         result, soa_rrset = finder.find(self._zone_name, RRType.SOA())
 > }}}
 >
 > Same for xfrout.

 I didn't know why the explicit default was there. It probably was before
 they were made optional, so I changed it now.

 > - ZoneFinder_helper_all: what's this?
 > {{{#!c++
 >     return Py_BuildValue("I", 1);
 > }}}
 >   This seems to be unreachable code (and more minor, it would need
 >   parentheses for the return value per coding guideline).  This is
 >   probably a mere copy of ZoneFinder_helper, and the same comment
 >   applies to it.

 Yes, it's copy-paste from the ZoneFinder_helper. I don't know the true
 purpose of it, but I guess some of our build machines got confused and
 thought it could reach the end of the function without calling return, so
 it was put there to prevent the warning. I could remove it and see if the
 build bots trigger it or not, if we wanted to try the theory out.

 > '''datasrc_test.py'''
 >
 > - as for the failure, I found it failed when I did 'make check' after
 >   'make clean'.  not looking into it further, but I guess there's some
 >   initialization issue.  This also suggests it would be better to
 >   confirm it passes distcheck if you didn't do it yourself.

 I usually run them both, but it seems I did forget this time somehow. Or
 maybe because it run on a tree where the read-write DB was already
 present. It gets copied in the setUp of the updater test class and this
 was by mistake in the finder test class. Moved (and common code got
 consolidated).

 > - I'd check if optional parameters can be actually omittable.

 It did check that. I added a comment where.

 > - if possible, I'd also test if the returned list and its elements
 >   would be cleaned up when they are not used (i.e., they don't have
 >   extra reference counts).

 I think the currently added test should do just that. The reference counts
 are counted manually, I don't see a way to check an object was really
 garbage collected in python.

 As for the changelog, yes, you're right, it's even backward incompatible
 API change.

 {{{
 [func]*         vorner
 The target parameter of ZoneFinder.find is no longer present, as the
 interface was awkward. To get all the RRsets of a single domain, use
 the new findAll method (the same applies to python version, the method
 is named find_all).
 }}}

 Thank you

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