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