BIND 10 #2851: extensions to ConfigurableClientList::getCachedZoneWriter

[BIND 10 Development]

#2851: extensions to ConfigurableClientList::getCachedZoneWriter
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130514
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:11 vorner]:

 > Few comments to the code. I didn't find a test testing reload after the
 zone was missing and then later it was added to the underlying data
 source. There's test checking it does not fail, but I don't think I
 noticed one that would try to add the zone and load it later.

 I've added a new test: `reloadNewZone`.  I think it addresses this
 point.

 > I believe this is either wrongly indented or tabs are present:

 Ah, there were tabs.  Not sure how they were introduced, but removed
 them anyway.

 > If the segment (cache) is not writable now, will it ever become writable
 by auth server? I believe it'll be written by the zone manager and never
 by auth.
 > {{{#!c++
 >         CACHE_NOT_WRITABLE, ///< The cache is not (yet) writable (and
 zones
 >                             ///  zones can't be loaded
 > }}}

 I meant by "yet" something like a shared-memory type cache, which is
 intended to be writable (so, in practice, for the memmgr) but the
 segment has not *yet* been set.  But as it rather confused you (a
 reader), and this level of details is not actually important for the
 API itself, I simply removed it.

 > This looks confused. The `DATASRC_NOT_FOUND` is not when the zone is
 missing in the named data source, but when the data source of the given
 name is not there.
 > {{{#!c++
 >     /// that data source is placed.  Even if the given name of zone
 doesn't
 >     /// exist in the data source, other data sources are not searched
 and
 >     /// this method simply returns DATASRC_NOT_FOUND in the first
 element
 >     /// of the pair.
 > }}}

 Oops, that was a simply typo, and should be ZONE_NOT_FOUND.  Fixed.

 > The docs for datasrc_name parameter is empty.

 Thanks for the catch, added the description.

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