BIND 10 #2203: separate CC Session from auth DataSourceConfigurator

[BIND 10 Development]

#2203: separate CC Session from auth DataSourceConfigurator
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121009
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  background zone loading            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:11 jelte]:
 > In general the changes look OK, and I'm fine with both the piggy-back
 change and the boost::function addition.

 Okay, good.

 > A problem though (kind of unrelated to this ticket itself, as it looks
 like this was already there); the rollback code upon '...' exception
 (datasrc_configurator.h:95) should not rethrow; as per current
 specification of addRemoteConfig the handler should never throw (the
 exception will fall through and auth will abort).
 >
 > Either we need to add another catch-all (+log error) in the code that
 calls the handler, or we need to log here, roll back, and not re-throw.
 The original reasoning of not catching in the caller of the handler was
 that that caller has, apart from logging an error, no real way of
 recovering from whatever caused this exception (and hence the handler
 should already be handling recovery and hence the exception should not
 even escape). We can discuss this, and we can defer it to another ticket,
 but we should certainly address it (as right now rolling back at this
 point is quite useless).

 We certainly need to discuss the topic of error handling (and
 recovery) in configuration, both for the "local" and remote cases.
 For example, as we discussed before, currently different modules can
 easily have inconsistent configuration when they share a single
 configuration.

 But, for this specific code I think we can defer this discussion.  In
 #2211 the remote config handler will be a simple message forwarder to
 the separate "configurator thread", so configureDataSourceGeneric()
 won't even be a part of the handler.  We'll probably still need to
 think about how to deal with exceptions from the data source level in
 terms of maintaining the thread with the possibility of seeing
 exceptions, but that will also be a topic of later tasks (probably
 #2210 and #2212).  For now, I'll just add a note in #2211 that the new
 handler should be exception free per API contract.

 Now, assuming it's okay not to touch this point, I made one final set
 of changes as mentioned in the previous comment: rename
 "datasrc_configurator" to "datasrc_config".  It should be trivial,
 but could you make sanity checks?

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