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      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:7 jinmei]:
 > I made one trivial editorial fix (coding guideline thing).
 >

 thanks

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

 might as well replace it now, so I did

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

 ok, removed

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

 oops. That TODO was for myself so i wouldn't forget to write it. Then I
 forgot...

 Did so now.

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

 They are documented, though perhaps not obviously, in the docstring for
 find().

 I did forget to add the new codes there though. Did so now.

 We may indeed want to (automatically) add such things explicitely to the
 type
 docstring at some point though.

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

 oh, yes, we needed to catch NotImplemented separately anyway (though this
 now reflects the original in that sense, i do wonder whether
 NotImplemented
 is the right exception for asking the previous name outside of the zone,
 but
 anyway)

 > '''about updater'''
 >
 > - probably not related to this ticket, but what's AZoneUpdater_find()?
 >   It doesn't seem to be used anywhere.

 dead code. removed.

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

 Ok, that's what I figured as well :)

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