BIND 10 #2213: revise LoadZoneCommand::exec() of b10-auth to use the configurator thread

[BIND 10 Development]

#2213: revise LoadZoneCommand::exec() of b10-auth to use the configurator thread
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121106
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  background zone loading            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:9 jinmei]:
 >
 > One way is to tighten the validation in the builder, but I personally
 > think it's better to catch it at the manager side because we can then
 > give the user (if it comes from a user via bindctl etc) an immediate
 > error report.
 >

 ah of course, the old errors don't bubble up anymore.

 I've added basic checking for the origin and class strings. There are
 however more scenarios that aren't caught; for starters if the zone (or
 class) itself isn't known in the configuration.

 We could add support for that (by factoring out the first part of the real
 threaded handler, that looks up the zone in the map), but it would mean
 doing that twice too (however, this may be relatively cheap anyway).

 Shall we do that too? (it should make error more reporting complete, up to
 the actual reading of data at least)

 > As for hardcoding as an intermediate workaround, I'm okay with it, but
 > I'd suggest leaving some comments about this point.
 >

 added

 > And, a relatively minor points:
 >
 > - maybe we should generalize `LoadZoneCommandError` so we can use it
 >   for all commands (in case we want to throw an error from them).

 I made it CommandError, and added a bit of doxy that it is supposed to be
 thrown if initial checks fail (i.e. before any actual command is sent)

 > - why do we need `AuthSrv::loadZone`?  I guess `LoadZoneCommand` can
 >   do `server.getDataSrcClientsMgr().loadZone()`.
 >

 ah doh, no we don't. Originally I didn't want to expose the whole clients
 mgr, and I had missed that this had been done anyway :p

 Removed.

 > BTW, I think we need a changelog entry (for the entire feature),
 > either as a separate one or by updating changelog no. 495.


 hmm yes.

 [func]  er, many devs :)
 The b10-auth 'loadzone' command now uses the internal thread introduced in
 495 to (re)load a zone in the background, so that query processing isn't
 blocked while loading a zone.
 (Trac #2213, git xxx)

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