BIND 10 #2781: Stats.do_polling should have direct tests

BIND 10 Development do-not-reply at isc.org
Tue Aug 27 08:13:10 UTC 2013


#2781: Stats.do_polling should have direct tests
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  vorner
            Priority:  medium        |                       Status:
           Component:  statistics    |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130903
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  4             |                 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 => vorner


Comment:

 Hello, thank you for reviewing.

 Replying to [comment:7 vorner]:
 > Why does this code use while and removing items from the list instead of
 for? And, even if while would be needed, the len is not needed, you can
 use a list in a boolean expression and it is true if non-empty.
 >
 > {{{#!python
 >         _sequences = sequences[:]
 >         while len(_sequences) > 0:
 >             (module_name, seq) = (None, None)
 >             try:
 > }}}

 I've used for-loop instead. Could you look at this?
 {{{
 c347349 [2781] use for-loop instead of while-loop for avoiding from
 rewriting the iterator
 }}}

 > This description doesn't match what is actually being done, because
 there's `assertRaises` at the end, not `assertEqual([], …)`.
 >
 > {{{#!python
 >     def test_get_multi_module_list_initsessiontimeout(self):
 >         """Test _get_multi_module_list() returns an empty list if
 rcp_call()
 >         raise a InitSeeionTimeout exception"""
 > }}}

 I mistook. I've corrected documentation and the code.
 {{{
 5207d19 [2781] correct description of
 test_get_multi_module_list_initsessiontimeout()
 }}}

 > This class seems odd. Is there a reason why the method is created by
 lambda and not by `def`? And, why it isn't actually just a dict with data
 and a complete class is needed?
 >
 > {{{#!python
 >         class DummyDict:
 >             items = lambda x: [
 >                 ('Init', 'dummy'), ('Stats', 'dummy'), ('Auth',
 'dummy'),
 >                 ]
 > }}}

 I've defined it as a normal function. But it is needed because the order
 of the array should be preserved at that code.
 {{{
 3669c95 [2781] define items() in DummDict class instead of using lambda,
 update note
 }}}

 > Also, it seems there's quite a lot of changes not directly related to
 the ticket. Not that I'd think the changes are for bad, but it might have
 been better to create a separate ticket for it. But I don't insist on
 doing that ex-post.

 As you know, do_polling() had various tasks as asking Init each module
 information, asking each module statistics, and collecting statistics. I
 think the scope of this ticket has become broad. Then some of changes I
 made might become things not directly related to this ticket. If you would
 like to have specific changes to be reviewed in separate tickets, please
 point out.

 Regards,

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


More information about the bind10-tickets mailing list