BIND 10 #1288: Update XFROUT to use new interface

[BIND 10 Development]

#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      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:14 vorner]:

 > > 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 ;-).

 Okay, I did it, but to make it workable I needed to merge this branch
 with master, which caused a lot of conflicts.  One of them is non trivial
 (the conflict resolution change in bind10_src.py.in at commit a6fd03e),
 so please carefully check that.

 > 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?

 Hmm, I couldn't reproduce it on my personal machine or on git.bind10
 (a Linux box).  So I have no clue.  Could you try to get more details
 about it?  For example, printing debug logs may help.

 >  * 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?

 Do you mean like this?
 {{{
         try:
             self.dns_xfrout_start(self._sock_fd, self._request_data,
 quota_ok)
         except Exception as e:
             logger.error(XFROUT_HANDLE_QUERY_ERROR, ex)
         finally:
             # Release any critical resources
             if quota_ok:
                 self._server.decrease_transfers_counter()
             self._close_socket()
 }}}

 I didn't do this because the `finally` part isn't executed if
 logger.error raises an exception.

 >  * 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)?

 It (at least in theory) couldn't because these are running on
 different threads, and a client object is not expected to be shared by
 multiple clients.  Also, I guess we'll revisit both the basic design
 of notify_out (it involves many threads in rather ad-hoc way, making
 the code very unreadable) and how to manage data source clients in
 this module (it should be more generic rather than hardcoding
 sqlite3).  At that point we can reconsider a cleaner and possibly more
 efficient way.  For now, I just added some comment about the rationale.

 I believe I addressed other comments.

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