BIND 10 #917: update stats-httpd to return only specified statistics

BIND 10 Development do-not-reply at isc.org
Mon Nov 7 17:50:59 UTC 2011


#917: update stats-httpd to return only specified statistics
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  naokikambe
  naokikambe                         |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20111108
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  statistics                         |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  6
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => naokikambe


Comment:

 Hello

 I see the same as Jeremy. I don't know why, but my browser shows only the
 headers of the table, but no context. It is OK in the overview page that
 shows everything.

 I fixed a small problem in the changelog text (and pushed), I hope you
 don't mind.

 The test fail for me, maybe you'll know more about why this might happen:
 {{{
 ======================================================================
 FAIL: test_do_GET (__main__.TestHttpHandler)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/home/vorner/work/bind10/src/bin/stats/tests/b10-stats-
 httpd_test.py", line 349, in test_do_GET
     check_XSL_URL_PATH(mod=None, item=None)
   File "/home/vorner/work/bind10/src/bin/stats/tests/b10-stats-
 httpd_test.py", line 300, in check_XSL_URL_PATH
     self.assertTrue(k in itm_vo)
 AssertionError: False is not true
 }}}

 Also, I have some less important notes about the code itself:
  * In this code, it looks unclean to decide by the text of exception:
 {{{#!python
 # Couldn't find neither specified module name nor
 # specified item name
 if str(err).startswith('Stats module: specified arguments are
 incorrect:'):
     self.send_error(404)
 else:
     self.send_error(500)
 }}}
    Would it be possible to throw inherited exception in that case?
  * This is quite obvious. However, it might be more helpful to note what
 the parameters mean/do.
 {{{
 the data which obtains from it. args are owner and name."""
 }}}
  * The closures (`get_stats_data`, `get_stats_spec`, …) don't really have
 a helpful docstring. It doesn't say what the functions actually do and
 that the last parameter is where the result is stored.
  * The closures ‒ it is not necessary to explicitly `return None`, if
 there's no return, the function returns None by itself.
  * Is it really OK to do this? Can't the conversion to us-ascii lose some
 characters? Can't there be non-ascii characters in the XML?
 {{{
 xml_string = str(xml.etree.ElementTree.tostring(xml_elem,
 encoding='utf-8'),
                  encoding='us-ascii')
 }}}
  * The recursive closures detect where they are in the structure by the
 type of parameter. Wouldn't it be better to write a separate function for
 each of the cases? Because we should be sure what type there is when we
 call it recursively.
  * The test commented as `# 404 NotFound (nonexistent item name)` ‒
 shouldn't it be tested on some module that exists? Because the module is
 nonexistent too.
  * Shouldn't assertEqual be used here?
 {{{#!python
 if item['item_type'] == 'list':
    self.assertTrue(len(item) == 7)
 else:
    self.assertTrue(len(item) == 6)
 }}}

 With regards

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


More information about the bind10-tickets mailing list