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

[BIND 10 Development]

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

 * owner:  muks => naokikambe


Comment:

 Hi Kambe san

 Here are my comments:

 * `counter.py` should be renamed to `counters.py`.
 * Multi-module tests should be added for the various scenarios in which
 the counters of a single spec file will be used, i.e, for cases similar to
 the `notifyoutv4` example (between `b10-xfrout` and `notify_out`) given in
 the `counter.py` module doc, and also for any other cases where such
 common counts will be maintained.
 * We should note in `__concat()`'s documentation that it is a helper
   function that is used to generate the keys.
 * I think we should remove the step argument from inc() and dec() and
   move the code to a common helper method. Exposing step to a caller as
   it is done currently is not good code. I have added a comment
 temporarily
   to both methods' docs that `step` should not be specified by the caller.
 * Is `dec()` only meant to be used for `axfr_running` and `ixfr_running`?
 * This is not a good API doc comment because you are exposing internal
 implementation details to the user of this module which that user may not
 know about (in order to follow the comment). It's simpler to just say
 "Starts a timer and keeps it running until stop_timer() is called.", and
 also talk about any other things that the caller should know. Same for
 `stop_time()`'s API doc too.
 {{{
         """Sets the value returned from _start_timer() as a value of the
         identifier in the self._start_time which is dict-type"""
 }}}
 * In the following API doc, do you mean returns instead of raises?
 {{{
         self._start_time after setting is successfully done. In case
         of stopping the timer which has never been started, it raises
         and does nothing. But in case of stopping the time which isn't
 }}}

 * I have pushed some comment updates to the `trac2225_statistics` branch.
 Please review them carefully and check if they are ok.

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