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