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