[2380] tighten option checks; make report interval configurable.

src/bin/loadzone/loadzone.py.in

@@ -54,15 +54,21 @@ the zone in.  Example:
 '{"database_file": "/path/to/dbfile/db.sqlite3"}'""",
                       metavar='CONFIG')
     parser.add_option("-d", "--debug", dest="debug_level",
-                      action="store", default=None,
-                      help="enable debug logs with the specified level",
-                      metavar='DEBUG_LEVEL')
+                      type='int', action="store", default=None,
+                      help="enable debug logs with the specified level")
+    parser.add_option("-i", "--report-interval", dest="report_interval",
+                      type='int', action="store",
+                      default=10000, # arbitrary choice
+                      help="""report logs progress per specified number of RRs
+(specify 0 to suppress report) [default: %default]""")
     parser.add_option("-t", "--datasrc-type", dest="datasrc_type",
                       action="store", default='sqlite3',
-                      help="type of data source (e.g., 'sqlite3')")
+                      help="""type of data source (e.g., 'sqlite3')\n
+[default: %default]""")
     parser.add_option("-C", "--class", dest="zone_class", action="store",
                       default='IN',
-                      help="RR class of the zone; currently must be 'IN'")
+                      help="""RR class of the zone; currently must be 'IN'
+[default: %default]""")
 
 class LoadZoneRunner:
     '''Main logic for the loadzone.
@@ -72,7 +78,6 @@ class LoadZoneRunner:
     '''
     def __init__(self, command_args):
         self.__command_args = command_args
-        self.__load_iteration_limit = 100000 # arbitrary choice for now
         self.__loaded_rrs = 0
 
         # system-wide log configuration.  We need to configure logging this
@@ -97,6 +102,7 @@ class LoadZoneRunner:
         self._datasrc_type = None
         self._log_severity = 'INFO'
         self._log_debuglevel = 0
+        self._load_iteration_limit = None
 
         self._config_log()
 
@@ -127,7 +133,12 @@ class LoadZoneRunner:
         # Configure logging policy as early as possible
         if options.debug_level is not None:
             self._log_severity = 'DEBUG'
+            # optparse performs type check
             self._log_debuglevel = int(options.debug_level)
+            if self._log_debuglevel < 0:
+                raise BadArgument(
+                    'Invalid debug level (must be non negative): %d' %
+                    self._log_debuglevel)
         self._config_log()
 
         if options.conf is None:
@@ -142,6 +153,12 @@ class LoadZoneRunner:
             raise BadArgument("RR class is not supported: " +
                               str(self._zone_class))
 
+        self._load_iteration_limit = int(options.report_interval)
+        if self._load_iteration_limit < 0:
+            raise BadArgument(
+                'Invalid report interval (must be non negative): %d' %
+                self._load_iteration_limit)
+
         if len(args) != 2:
             raise BadArgument('Unexpected number of arguments: %d (must be 2)'
                               % (len(args)))
@@ -175,6 +192,10 @@ class LoadZoneRunner:
             cur.execute("DELETE FROM zones WHERE name = ?",
                         [self._zone_name.to_text()])
 
+    def _report_progress(self, loaded_rrs):
+        logger.info(LOADZONE_LOADING, loaded_rrs,
+                    self._zone_name, self._zone_class)
+
     def _do_load(self):
         '''Main part of the load logic.
 
@@ -191,10 +212,12 @@ class LoadZoneRunner:
                             self._zone_class)
             loader = ZoneLoader(datasrc_client, self._zone_name,
                                 self._zone_file)
-            while not loader.load_incremental(self.__load_iteration_limit):
-                self.__loaded_rrs += self.__load_iteration_limit
-                logger.info(LOADZONE_LOADING, self.__loaded_rrs,
-                            self._zone_name, self._zone_class)
+            if self._load_iteration_limit > 0:
+                while not loader.load_incremental(self._load_iteration_limit):
+                    self.__loaded_rrs += self._load_iteration_limit
+                    self._report_progress(self.__loaded_rrs)
+            else:
+                loader.load()
         except Exception as ex:
             # release any remaining lock held in the client/loader
             loader, datasrc_client = None, None

src/bin/loadzone/tests/loadzone_test.py

@@ -67,6 +67,7 @@ class TestLoadZoneRunner(unittest.TestCase):
         self.assertIsNone(self.__runner._zone_file)
         self.assertIsNone(self.__runner._datasrc_config)
         self.assertIsNone(self.__runner._datasrc_type)
+        self.assertIsNone(self.__runner._load_iteration_limit)
         self.assertEqual('INFO', self.__runner._log_severity)
         self.assertEqual(0, self.__runner._log_debuglevel)
 
@@ -76,6 +77,7 @@ class TestLoadZoneRunner(unittest.TestCase):
         self.assertEqual(NEW_ZONE_TXT_FILE, self.__runner._zone_file)
         self.assertEqual(DATASRC_CONFIG, self.__runner._datasrc_config)
         self.assertEqual('sqlite3', self.__runner._datasrc_type) # default
+        self.assertEqual(10000, self.__runner._load_iteration_limit) # default
         self.assertEqual(RRClass.IN(), self.__runner._zone_class) # default
         self.assertEqual('INFO', self.__runner._log_severity) # default
         self.assertEqual(0, self.__runner._log_debuglevel)
@@ -92,8 +94,9 @@ class TestLoadZoneRunner(unittest.TestCase):
                           LoadZoneRunner(['example', 'example.zone']).
                           _parse_args)
 
+        copt = ['-c', '0']      # template for the mandatory -c option
+
         # There must be exactly 2 non-option arguments: zone name and zone file
-        copt = ['-c', '0']
         self.assertRaises(BadArgument, LoadZoneRunner(copt)._parse_args)
         self.assertRaises(BadArgument, LoadZoneRunner(copt + ['example']).
                           _parse_args)
@@ -114,12 +117,24 @@ class TestLoadZoneRunner(unittest.TestCase):
                           LoadZoneRunner(copt + ['-C', 'CH']).
                           _parse_args)
 
+        # bad debug level
+        args = copt + ['example.org', 'example.zone'] # otherwise valid args
+        self.assertRaises(BadArgument,
+                          LoadZoneRunner(['-d', '-10'] + args)._parse_args)
+
+        # bad report interval
+        self.assertRaises(BadArgument,
+                          LoadZoneRunner(['-i', '-5'] + args)._parse_args)
+
     def __common_load_setup(self):
         self.__runner._zone_class = RRClass.IN()
         self.__runner._zone_name = TEST_ZONE_NAME
         self.__runner._zone_file = NEW_ZONE_TXT_FILE
         self.__runner._datasrc_type = 'sqlite3'
         self.__runner._datasrc_config = DATASRC_CONFIG
+        self.__runner._load_iteration_limit = 1
+        self.__reports = []
+        self.__runner._report_progress = lambda x: self.__reports.append(x)
 
     def __check_zone_soa(self, soa_txt, zone_name=TEST_ZONE_NAME):
         """Check that the given SOA RR exists and matches the expected string
@@ -148,8 +163,26 @@ class TestLoadZoneRunner(unittest.TestCase):
         self.__common_load_setup()
         self.__check_zone_soa(ORIG_SOA_TXT)
         self.__runner._do_load()
+        # In this test setup every loaded RR will be reported, and there will
+        # be 3 RRs
+        self.assertEqual([1, 2, 3], self.__reports)
         self.__check_zone_soa(NEW_SOA_TXT)
 
+    def test_load_update_skipped_report(self):
+        '''successful loading, with reports for every 2 RRs'''
+        self.__common_load_setup()
+        self.__runner._load_iteration_limit = 2
+        self.__runner._do_load()
+        self.assertEqual([2], self.__reports)
+
+    def test_load_update_no_report(self):
+        '''successful loading, without progress reports'''
+        self.__common_load_setup()
+        self.__runner._load_iteration_limit = 0
+        self.__runner._do_load()
+        self.assertEqual([], self.__reports) # no report
+        self.__check_zone_soa(NEW_SOA_TXT)   # but load is completed
+
     def test_create_and_load(self):
         '''successful case to loading contents to a new zone (created).'''
         self.__common_load_setup()