BIND 10 #1329: Extend DatabaseAccessor to support adding diffs

[BIND 10 Development]

#1329: Extend DatabaseAccessor to support adding diffs
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111108
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:         |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jelte):

 Replying to [comment:9 jinmei]:
 > Replying to [comment:8 jelte]:
 >
 > > (very general note; we seem to be pretty inconsistent in using !///
 and
 > > /!** notation for doxygen comments, even within single files.
 Certainly
 > > unrelated to this ticket, but perhaps we want to make this a bit more
 > > consistent. Especially for instance in database.h where the comment
 > > for DiffOperation uses !///, the code above and below it use /!**, and
 > > at some point everything is !/// again)
 >
 > Yeah, I know we mix the two styles.  I personally would like to make
 > them more consistent, but has been ignoring this point as it might be
 > a pure bikeshed.  (Note also that we allow both styles in our coding
 > guideline).  But it may make sense to introduce some guideline at
 > least inside each single file and/or for doxygen comments.  Maybe
 > discuss this at the next biweekly call?
 >

 ok, i'll mention it to shane

 >
 > > Should we have some way of signaling whether journaling is supported
 > > by the datasource at all?
 >
 > I think we should.  I'm not 100% sure what is the best way to do this.
 > Unlike the case for updater and iterator we cannot signal this at the
 > construction time.  I'd be adding a general interface to the
 > `DataSourceClient` class to return specific capabilities (writable,
 > iteratable, journaling, and perhaps more) of the underlying data
 > source.  xfrin or dynamic update server can check this before creating
 > an updater, and specify journaling only when it's supported.
 > (Should we create a ticket for this?)
 >

 yeah, created ticket #1382 for this

 > Whether or not do this, it would also make sense to allow the
 > implementation of addRecordDiff to signal it when journaling isn't
 > supported.  So I updated the documentation saying it will throw
 > `NotImplemented` in such a case (no test for this because it's
 > supported SQLite3).
 >

 ok, cool

 > > Just one real comment, in sqlite3_accessor.cc, and about a (probably)
 > > temporary method anyway. So if we plan to ditch it, maybe we should
 > > only add some comments in the cc file regarding the potential issues
 > > below (in case it is taken as the base for whatever we write later).
 > > Or you can ignore the comments below here :)
 >
 > You're right, and you also rightly guessed that it was intended to be
 > ditched.  So I'd leave the code as is, but add some more comments
 > about the intent.

 ok.

 All good, please merge :)

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