BIND 10 #1252: Datasource refactor finishing touch 2: wrappers/NSEC support

[BIND 10 Development]

#1252: Datasource refactor finishing touch 2: wrappers/NSEC support
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jinmei
                       Type:  task   |                Status:  reviewing
                   Priority:  minor  |             Milestone:
                  Component:  data   |  Sprint-20111011
  source                             |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DNS
Feature Depending on Ticket:         |  Estimated Difficulty:  2
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I made one trivial editorial fix (coding guideline thing).

 '''datasrc.cc'''

 This is no worse than the existing code:-), but addClassVariable()
 is (at least IMO) error prone.  For newer code I'd use
 installClassVariable() defined in lib/util/python/pycppwrapper_util.h
 in a try-catch block.

 Maybe it's beyond the scope of this ticket, but in order to prevent
 (or at least discourage) further use of addClassVariable() due to
 copy-paste, we should probably at least rename it to something like
 obsoletedAndDoNotUseForNewCodeAddClassVariable() until we replace
 everything with safer interfaces.

 '''finder_python'''
 - I believe we can avoid reinterpret_cast here:
 {{{
     { "find_previous_name",
 reinterpret_cast<PyCFunction>(ZoneFinder_findPreviousName),
       METH_VARARGS, ZoneFinder_find_doc },/*TODO DOC*/
 }}}
 - As for "TODO DOC": Is it really a TODO?  I suspect it's quite easy
   to generate (though you'll need to make adjustments) from the C++
   doc with the semi-auto-generator script.  In any case, I don't think
   referring to ZoneFinder_find_doc is appropriate.
 - Also about documentation, it would be better if we could provide doc
   for finder result codes (the generator script may not have a support
   for enums, though)

 '''datasrc_test'''

 - Unless I don't misunderstand it findPrevious() throws if it's given
   a smaller name than zone's origin.  I guess this case should be
   tested too.

 '''about updater'''

 - probably not related to this ticket, but what's AZoneUpdater_find()?
   It doesn't seem to be used anywhere.
 - as for whether to define findPreviousName() for the updater, I'm not
   sure - in practice we probably don't need it.  The updater's finder
   is provided so that the application can perform post-update checks
   within the transaction, and for that purpose it would be sufficient
   for a very primitive form of lookup (we probably don't even need
   GLUEOK etc).  On the other hand it would be nicer to provide it
   for completeness.  My slight preference here is to skip it for now
   in the sense of YAGNI unless/until we see the real need for it.

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