BIND 10 #2380: revise b10-loadzone using datasrc.ZoneLoader

[BIND 10 Development]

#2380: revise b10-loadzone using datasrc.ZoneLoader
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  loadzone      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20121218
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  8             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:9 jelte]:

 > While it is probably the most flexible approach, I don't think we should
 force people to enter JSON on the command line in order to load a zone...

 Yeah I know...

 > Perhaps we can also add an option to only specify an sqlite3 file and
 create the config magic ourselves (leaving the current option for other
 datasources), e.g. has a -s <sqlite3-db-file> if -t is sqlite3 or not
 given (a bit similar to how logging 'config' is done now, and for the
 user, similar to the old -d option).

 I really wanted to avoid that because it looked too ad hoc.  Basically
 I'd like to keep the user interface of this tool datasrc-type
 agnostic.  Also,

 > And/or, but that may be something to add later, if it can find the
 'global' bind10 configuration file, it could in theory figure out if there
 is something configured (and, say, pick the first one that supports
 loading) but in the near future this would also require a hackish
 approach, as it would have to untangle the json file itself. Regarding
 that, if this replaces the existing loadzone now, the TODO file is not
 completely right).

 I believe this is the way to go in a longer term, and/or, if there's
 not yet a global config, I believe we should still keep the tool
 independent from data source types, but allow it to get config info
 via some kind of plugin mechanisms, maybe interactively.

 For the temporary measure for the beta release, I propose this: allow
 omitting -c for sqlite3 data source (which is the only supported data
 source at the moment anyway).  When omitted, it internally uses the
 configuration with specifying the default sqlite3 DB file.  It's still
 datasrc-specific hack hardcoded in the source, but at least the user
 interface is still datasrc-agnostic.

 I've implemented this proposed approach.

 > On another topic, ye olde loadzone printed how many RRs were loaded, and
 how long it took. I realize the current API does not support the former
 well, but I wonder if it would be worth it to bring that back. It's kind
 of nice to affirm you're not suddenly loading a 2-record zone :) It still
 does so if you use -i 1, but unless specifically disabled, I'd love to see
 the final result report (mainly, the count for the last 'iteration' is
 obviously missing for any value > 1).

 I've tried to improve it a bit.  But that's not a perfect as you
 noticed, of course.  We should create a ticket for getting the total
 number of loaded RRs and use it.  We should also be able to print
 the progress in percentage.  These require extensions to the
 underlying API, and will consist of a few tickets.

 > In the usage() output (optparse help), we should probably add something
 like [0-99] for the debug level option (and come to think of it, perhaps a
 silent mode, or rather, only WARN and ERROR).

 Added [0-99].  For further suppression, I'd like to defer it to a
 later ticket at this stage.

 > Tests fail for me (I build with --prefix but without installing):
 > {{{
 > isc.datasrc.Error: Failed to create DataSourceClient of type
 sqlite3:dlopen failed for
 /home/jelte/opt/bind10/libexec/bind10-devel/backends/sqlite3_ds.so:
 /home/jelte/opt/bind10/libexec/bind10-devel/backends/sqlite3_ds.so: cannot
 open shared object file: No such file or directory
 > }}}
 >
 > (a whole bunch of them and other errors, all of which I assume because
 of this)
 >
 > I guess the Makefile needs to set B10_FROM_BUILD to get the correct
 dynamic loading dir.

 Ah, right, added.

 > loadzone.py.in:
 >
 > is the if necessary here?
 > {{{
 >             if self._load_iteration_limit > 0:
 >                 limit = self._load_iteration_limit
 >             else:
 >                 limit = LOAD_INTERVAL_DEFAULT
 > }}}
 > ( !__init!__ already defaults to LOAD_INTERVAL_DEFAULT, as does the
 optparse code, so it certainly looks like it should be able to always use
 self._load_iteration_limit)

 It is actually necessary, but I admit it wasn't clear.  I tried to
 clarify that by renaming a confusing variable and adding some more
 comments.

 > Overwriting the same line with \r in !__report_progress() is cute,
 however, we do need a \n somewhere :) (probably before if
 self.!__interrupted around line 240)

 Okay, added.

 > maybe a matter of taste, but should we not consider a zone without a SOA
 a complete failure and error/revert on it?

 We should, but I'd like to defer it to the forcecoming validator (that
 will run under the updater, so the rollback will automatically happen
 once we implement validator and integrate it in the zone loader).  We
 could make this cleanup as a temporary hack here, but that would be
 much more complicated than just canceling adding the zone, so I opted
 to not do too much here.

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