BIND 10 #191: Implement a initial version of stats

BIND 10 Development do-not-reply at isc.org
Fri Aug 27 01:29:55 UTC 2010


#191: Implement a initial version of stats
-----------------------------+----------------------------------------------
      Reporter:  naokikambe  |        Owner:  jinmei              
          Type:  task        |       Status:  reviewing           
      Priority:  major       |    Milestone:  y2 6 month milestone
     Component:  statistics  |   Resolution:                      
      Keywords:              |    Sensitive:  0                   
Estimatedhours:  0.0         |        Hours:  3.08                
      Billable:  0           |   Totalhours:                      
      Internal:  0           |  
-----------------------------+----------------------------------------------
Changes (by jinmei):

  * hours:  0.0 => 3.08
  * totalhours:  0.0 => 3.08


Comment:

 I've not completed my review, but am providing some initial feedback:

 '''run_b10-stats.sh.in'''
  - is B10_FROM_BUILD necessary? (the same comment applies to other .sh)

 '''stats.spec.pre.in'''
  - I don't think this is a good "default". (it rather looks like an
 "example")
 {{{
         "item_default": "123abc at xxxx",
 }}}

 '''unittest_fakesession.py'''
  - this seems to be largely derived from
 config/tests/unittest_fakesession.py.  please note it if you do something
 like that.  it will help reduce review burden very much.  also, it's even
 better to unify the code (with appropropriate abstraction if per-module
 customization is necessary).  having duplicate code is generally not a
 good practice.
  - I'm not sure if this line of close() matches the what this method does:
 {{{
         # need to pass along somehow that this function has been called,
 }}}
   moreover, I'm not even sure about the purpose of this method.  Is this
 while-select trick necessary?  Please describe the purpose of this method
 and how it's expected to be used (as comments).

  - this line in close() seems to be dead code:
 {{{
                 raise Exception('Unexpected return values from select():',
 r, w, e)
 }}}
  - test_boss_stub: I don't this is a good test because get_datetime() may
 return different values in the test and in the testee.  (code duplication
 of get_datetime() is another bad thing)

 '''b10-stats_stub_test.py'''
  - I'd move tearDown() right after setUp().

 '''stats_stub.py.in'''
  - BossModuleStub.send_command(): the method name is too generic for what
 it's actually doing.
  - I suggest all temporary resources be cleaned up in tearDown().  from a
 quick look, it seems self.listener has to be stopped.

 '''b10-stats_test.py'''
  - the intent of this part is not clear.  please add comment:
 {{{
         for i in (0, 1):
             self.session.get_message("ConfigManager", None)
 }}}

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


More information about the bind10-tickets mailing list