BIND 10 #2835: add interface to get properties of datasrc clients from ClientList

BIND 10 Development do-not-reply at isc.org
Wed Mar 27 05:56:21 UTC 2013


#2835: add interface to get properties of datasrc clients from ClientList
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130402
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 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):

 Replying to [comment:14 vorner]:

 I think the branch is almost ready for merge.  I made a few trivial
 fixes, and have a couple of minor suggestions:

 - I suggest clarifying the intended semantics of "segment type" in
   the `getSegmentType` documentation, e.g.:
 {{{#!cpp
     /// \brief Get the segment type
     ///
     /// \note Specific values of the type are only meaningful for
 corresponding
     /// memory segment implementation and modules that directly manage the
     /// segments.  Other normal applications should treat them as opaque
     /// identifiers.
     ///
     /// \throw isc::InvalidOperation if called and state is
 SEGMENT_UNUSED.
     const std::string& getSegmentType() const {
 }}}
 - In DataSourceStatus::status, my previous comment on the comment
   doesn't seem to be addressed:
 {{{#!cpp
 // Check the status holds data and can change the segment state
 TEST(DataSourceStatus, status) {
 }}}
   I suggest simply removing "and can change the segment state".

 I don't think I need to check fixes to them.  Please apply with any
 minor modifications as you like, and merge it.

 Some followup comments:

 > > - IIRC, an optional item must have a default (otherwise, e.g., bindctl
 > >   doesn't work correctly with it), although I may be wrong - the
 > >   concept of "optional" has always confused me.
 >
 > I couldn't test (I'm using a very slow netbook, and I don't have the
 time to
 > compile full bind10 ☹). But, from logical point of view, a default makes
 no
 > sense with optional item.

 Right, that's why it has confused me:-), but I now checked it with
 bindctl and confirmed it works without defining the default.  So I may
 memory was incorrect on this or it was fixed as you said.  In either
 case this is not a blocking issue for this branch.

 > > - `MemorySegmentType`: I'm not sure if we define them here.  In my
 > >   image specific details of segment types are private to memory
 > >   segment (and corresponding `ZoneTableSegment`) implementations, and
 > >   the `ClientList` class uses it transparently.  So, for example, we
 > >   might allow introducing another type of memory segment as a plugin
 > >   in future.  For that reason my original intent is to define the type
 > >   in a semantics-free type, such as a plan std::string.
 >
 > Hmm. Passing these kinds of things as strings doesn't feel right either,
 but it
 > might be the lesser evil.

 Yeah bear strings have some drawbacks, most notably the resulting
 interface is less type safe.  It would be cleaner, e.g., if we
 introduce a dedicated (base) class to represent types and let
 implementations define derived class to represent its type, but in
 this case it would probably be overkilling.

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


More information about the bind10-tickets mailing list