BIND 10 #2252: Implement counters into Xfrin (1/3)

[BIND 10 Development]

#2252: Implement counters into Xfrin (1/3)
-------------------------------------+-------------------------------------
            Reporter:  naokikambe    |                        Owner:
                Type:  enhancement   |  jinmei
            Priority:  medium        |                       Status:
           Component:  xfrin         |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130423
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by naokikambe):

 * owner:  naokikambe => jinmei


Comment:

 Hello,

 Replying to [comment:24 jinmei]:
 > > > '''xfrin.py.in'''
 > > >
 > > > - In this revised code, if IXFR results in `XfrinZoneUptodate`
 xfrfail
 > > >   is counted.  Is it intentional, and if so, is it reasonable?  From
 a
 > > >   quick look, BIND 9 seems to consider it a success.  In any case,
 > > >   this should be clarified in the documentation: what exactly
 > > >   "succeed/fail" mean.
 > >
 > > I revised the codes to treat an xfr request as success in a case of
 raised `XfrinZoneUptodate`. For consistency between BIND 9 and 10, I think
 we should define this case as success. In fact, `do_xfrin()` already
 defines so. The commit is:
 > > {{{
 > > 8544c8e [2252] treat an xfr request as success with an
 XfrinZoneUptodate raised
 > > }}}
 >
 > Documentation update is missing.

 Documentation was not same but I've updated a bit. The commit is:
 {{{
 4b79543 [2252] update documentation for the test cases
 }}}

 > And one minor point: this comment doesn't parse well to me:
 > {{{
 >                 # Note: Catching an exception XfrinZoneUptodate
 >                 # defines that IXFR succeeded.
 > }}}

 In the below change I've removed the inner try-except statement. So the
 note was unnecessary.

 > And, one last non trivial point.  I'd hesitate to say this after the
 > long cycle of review process, but do_xfrin() now looks overly
 > complicated to me due to the series of statistics related changes.
 > The organization of the nested try-re-raise-catch is particularly
 > unreadable to me.  I suggest refactoring this method so we can
 > understand the entire flow is more easily.  And, I wouldn't like to
 > defer it to a separate ticket; it'll quite likely to be buried in
 > other tickets.

 I've tried to refactor it so that it's easy to read. The commit is:
 {{{
 31f138b [2252] simplify complicated nested try-except statements
 }}}

 Regards,

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