BIND 10 #756: Python logging - produce API

[BIND 10 Development]

#756: Python logging - produce API
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110628
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  logging                            |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => vorner


Comment:

 I've pushed a minor change with a few trivial typo fixes, see
 576f337f42e5c72aec48d8ea1d36ab5059588301

 One general thing, for the python dns wrappers, we put the code in
 src/lib/dns/python, as opposed to src/lib/python/isc/dns. Not sure which
 one is better (it is c++ code, so src/lib/<lib>/python makes sense, but it
 is for python, so src/lib/python/isc/<lib>/ also makes sense). We can
 probably move one or the other later, but we may want to discuss which to
 choose for consistency.

 src/lib/python/isc/log/__init__.py:

 just wondering, does the pythonpath setting in the make check target not
 work or is that not enough?

 src/lib/python/isc/log/log.cc:

 perhaps if you want to enforce that set_test_dictionary() gets a bool you
 could use PyBool_Check();
 for instance like this, it will throw if the argument is not True or
 False:
 {{{
     PyObject* enableO;
     bool enable = false;
     if (PyArg_ParseTuple(args, "O", &enableO) &&
         PyBool_Check(enableO)) {
         if (PyObject_IsTrue(enableO)) {
             enable = true;
         }
     } else {
         PyErr_SetString(PyExc_TypeError,
                         "argument to set_test_dictionary() should be True
 or False");
         return (NULL);
     }
 }}}

 To me, that is a tiny bit clearer, though it has another problem (which
 can be fixed by splitting the ifs), and the existing code is fine too,
 just wanted to mention that there is in fact a way to check bool type
 (even though there is no real bool type, just true and false)

 reset():
 (regarding the comment) you don't need to check whether there are no
 arguments here, as the methods array already specifies METH_NOARGS for
 this one (so it'll raise a type error if any arguments are given)


 severityToText():
 Should we move severityToText() to the cpp api? (we do have text to
 severity there)

 As discussed on jabber, perhaps we should prepend those testing/internal
 functions in the module with a _.

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