BIND 10 #2222: Implement counters into Xfrout (2/3)

[BIND 10 Development]

#2222: Implement counters into Xfrout (2/3)
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  naokikambe
  naokikambe                         |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20120925
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  xfrout                             |  Estimated Difficulty:  4
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => naokikambe


Comment:

 Hello

 I'm not sure if I lack python experience, and I'm not at my best today
 generally. But I find a lot of the code there very hard to read or
 completely
 unreadable.

 These are hard to read. Would it be possible to simplify it somehow? Or at
 least comment what is happening and how?
 {{{#!python
             incrementer = \
                 dict(self.xfrout_counter.get_counters_for_xfroutsession(),
 \
 **self.xfrout_counter.get_counters_for_notifyout())\
                          ['counter_%s' % counter_name]
 }}}

 As well as these (I'd suggest using full-blown if-else here):
 {{{#!python
             self._statistics_data[n] \
             if n.find('ixfr_') == 0 or n.find('axfr_') == 0 \
             else self._statistics_data['zones'][TEST_ZONE_NAME_STR][n]
 }}}

 And this is just too much for me, I have no clue what this code does. The
 comment about argument does not help a bit, I have no idea what argument
 it
 talks about:
 {{{#!python
         # Set counter handlers for counting Xfr requests or
         # incrementing or decrementing Xfr running. An argument
         # is required for zone name in counting Xfr requests.
         for (k, v) in counters.items():
             if k.find('counter_') == 0 or k.find('inc_') == 0 \
                     or k.find('dec_') == 0:
                 setattr(self, "_%s" % k, v)
 }}}

 Also, some tests seem to have quite a lot of levarage. I saw similar one 3
 times I think. Could the Greater be equal to 1? Because it should increase
 the
 counter by exactly one transfer:
 {{{#!python
          self.assertEqual(self.get_counter('xfrrej'), 0)
          self.check_transfer_acl(acl_setter)
          self.assertGreater(self.get_counter('xfrrej'), 0)
 }}}

 And, does this test check anything, or is it completely useless? It
 loosely
 translates to „the counter may or may not have been incremented. Whatever
 the
 server likes.“
 {{{
     And when I query statistics axfr_running of bind10 module Xfrout with
 cmdctl port 47804
     # xfering might not be started yet.
     Then the statistics counter axfr_running should be between 0 and 1
 }}}

 And, also, it is mostly aesthetics, but the xxcrementer used at few places
 sounds very much like excrementer, and while I don't think anyone would
 ever use
 that word in English, it quite undoubtedly means a person sitting
 on a toilet at the moment. I'm not sure how conservative we are in naming
 things, but this looks like too much amusing name compared with the rest
 of the
 code O:-).

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