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

BIND 10 Development do-not-reply at isc.org
Tue Nov 8 08:28:00 UTC 2011


#917: update stats-httpd to return only specified statistics
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  UnAssigned
  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 naokikambe):

 * owner:  naokikambe => UnAssigned


Comment:

 Hello,
 Replying to [comment:10 vorner]:
 > I fixed a small problem in the changelog text (and pushed), I hope you
 don't mind.
 Thank you very much.

 > 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
 > }}}

 The path names of the XSL/XHTML structure in the !ElementTree obejct
 were incorrect. The path name ended with the unnecessary slash. I
 already fixed them at git f8a64959bc5f3ddf68ba4d01bee092bf4f1f9558. Please
 see it.

 >  * 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?

 I added a new exception !StatsHttpdDataError. It is raised when an
 error due to the specified data is occurred. If it's raised,
 stats-httpd will return 404 notfound. Please see git
 df02b63fe1176c572a7eee996921f211ca970953.

 >  * 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.

 I added the more docstrings. Please see git
 8e8607c6faa34d9493a831054ecb64281f1f06c7 and git
 4ab7d17edc10ce4f7b834709aa009aba4db9d877. If they are not enough, please
 let me know.

 >  * The closures - it is not necessary to explicitly `return None`, if
 there's no return, the function returns None by itself.

 I removed such {{{return None}}}s. Please see git
 50e96053742a30584f91a6bdb4b788977cd166bf.

 >  * 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')
 > }}}

 Yes, it loses non-ascii characters. That was added in #1021 but that
 might be a work around. However I think that such non-ascii characters
 wouldn't be acceptable in the system. If we make that acceptable, we
 have to totally review the specification of the whole system. I know
 such non-ascii characters are urgent into the future, but do we need
 it now?

 >  * 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.

 Such internal functions are temporary in this ticket. I'd like to
 this task would be bigger. Sorry, please give me any suggestions if
 you have.

 >  * 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.

 I added such testcases. Please see git
 cc48074a9fec60ef9ba69991549f9e167e620225.

 >  * Shouldn't assertEqual be used here?
 > {{{#!python
 > if item['item_type'] == 'list':
 >    self.assertTrue(len(item) == 7)
 > else:
 >    self.assertTrue(len(item) == 6)
 > }}}

 I fixed it. Please see git f1e08d75cabc45454a9bde86158dc8c7348d7f9d.

 Best

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


More information about the bind10-tickets mailing list