BIND 10 #1288: Update XFROUT to use new interface

BIND 10 Development do-not-reply at isc.org
Fri Nov 11 13:17:51 UTC 2011


#1288: Update XFROUT to use new interface
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111122
                  Component:         |            Resolution:
  xfrout                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:9 jinmei]:
 > The branch resulted in a bit bigger than I expected, partly because
 > I didn't realize I would need to updte notify_out, too.  Here are some
 > hints for the reviewer that hopefully mitigates the review load:

 That's OK, I've seen worse O:-).

 Replying to [comment:13 jelte]:
 > BTW, #1217 has been merged, so we should either add True to the call to
 get_iterator() (and update the Mock class) when merging this, or create a
 (very minor) ticket to do so, so that TTLs are kept upon xfrout.

 I believe this might be better to do now, as it is really small. Creating
 a ticket would be more work than the actual coding ;-).

 So, about the review:
  * For some reason I did not pursue, the tests segfault (yes, in python):
 {{{
 TESTDATASRCDIR=/home/vorner/work/bind10/src/lib/python/isc/notify/tests/testdata/
 \
 /usr/bin/python3
 /home/vorner/work/bind10/src/lib/python/isc/notify/tests/$pytest || exit ;
 \
 done
 Running test: notify_out_test.py
 E/bin/sh: line 1: 24806 Segmentation fault      (core dumped)
 PYTHONPATH=/home/vorner/work/bind10/src/lib/python/isc/log_messages:/home/vorner/work/bind10/src/lib/python:/home/vorner/work/bind10/src/lib/python:/home/vorner/work/bind10/src/lib/dns/python/.libs
 TESTDATASRCDIR=/home/vorner/work/bind10/src/lib/python/isc/notify/tests/testdata/
 /usr/bin/python3
 /home/vorner/work/bind10/src/lib/python/isc/notify/tests/$pytest
 make[8]: Leaving directory
 `/home/vorner/work/bind10/src/lib/python/isc/notify/tests'
 }}}
     Should I try to look what happens or can you reproduce or guess?
  * Looking at this test function:
 {{{#!python
     def get_soa(self):  # emulate ZoneIterator.get_soa()
         if self._zone_name == Name('nosoa.example.com'):
             return None
         soa_rrset = RRset(Name('multisoa.example.com'), RRClass.IN(),
                           RRType.SOA(), RRTTL(3600))
         soa_rrset.add_rdata(Rdata(RRType.SOA(), RRClass.IN(),
                                   'master.example.com. ' +
                                   'admin.example.com. 1234 ' +
                                   '3600 1800 2419200 7200'))
         if self._zone_name == Name('multisoa.example.com'):
             soa_rrset.add_rdata(Rdata(RRType.SOA(), RRClass.IN(),
                                       'master.example.com. ' +
                                       'admin.example.com. 1300 ' +
                                       '3600 1800 2419200 7200'))
             return soa_rrset
         return soa_rrset
 }}}
     Doesn't it return a multisoa.example.com SOA even if a different one
 is asked? Shouldn't it be `RRset(self._zone_name, …`?
  * The same function, last two lines ‒ the return is the same, so the one
 inside the condition isn't needed, is it?
  * A docstring in tests, typo (teraDown):
 {{{
 We just check it doesn't any unexpected disruption and (in teraDown)
 }}}
  * Should the `test_check_xfrout_available` check a successful case?
  * There's a try-except in the `_handle` method where the exception is
 stored and left aside for a while. But, wouldn't a try-except-finally be
 better?
  * In the notifications, it constructs a client twice, once to get the NS,
 once to get the SOA. Couldn't it be done with a single client (and
 finder)?

 Thanks

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


More information about the bind10-tickets mailing list