[2573] updated python wrapper using loader's getProgress.

get_size/get_position were deprecated and removed.

src/lib/python/isc/datasrc/datasrc.cc

@@ -21,6 +21,7 @@
 #include <datasrc/client.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
+#include <datasrc/zone_loader.h>
 
 #include "datasrc.h"
 #include "client_python.h"
@@ -34,6 +35,9 @@
 #include <util/python/pycppwrapper_util.h>
 #include <dns/python/pydnspp_common.h>
 
+#include <stdexcept>
+#include <string>
+
 using namespace isc::datasrc;
 using namespace isc::datasrc::python;
 using namespace isc::util::python;
@@ -195,6 +199,21 @@ initModulePart_ZoneLoader(PyObject* mod) {
     }
     Py_INCREF(&zone_loader_type);
 
+    try {
+        installClassVariable(zone_loader_type, "PROGRESS_UNKNOWN",
+                             Py_BuildValue("i", ZoneLoader::PROGRESS_UNKNOWN));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in ZoneLoader initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Unexpected failure in ZoneLoader initialization");
+        return (false);
+    }
+
     return (true);
 }
 

src/lib/python/isc/datasrc/tests/zone_loader_test.py

@@ -38,7 +38,7 @@ ORIG_SOA_TXT = 'example.com. 3600 IN SOA master.example.com. ' +\
                'admin.example.com. 1234 3600 1800 2419200 7200\n'
 NEW_SOA_TXT = 'example.com. 1000 IN SOA a.dns.example.com. ' +\
               'mail.example.com. 1 1 1 1 1\n'
-
+PROGRESS_UNKNOWN = isc.datasrc.ZoneLoader.PROGRESS_UNKNOWN
 
 class ZoneLoaderTests(unittest.TestCase):
     def setUp(self):
@@ -112,16 +112,14 @@ class ZoneLoaderTests(unittest.TestCase):
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.test_file)
         self.assertEqual(0, self.loader.get_rr_count())
-        self.assertEqual(0, self.loader.get_size())
-        self.assertEqual(0, self.loader.get_position())
+        self.assertEqual(0, self.loader.get_progress())
 
         self.check_load()
 
         # Expected values are hardcoded, taken from the test zone file,
-        # assuming it won't change too often.
+        # assuming it won't change too often.  progress should reach 100%.
         self.assertEqual(8, self.loader.get_rr_count())
-        self.assertEqual(422, self.loader.get_size())
-        self.assertEqual(422, self.loader.get_position())
+        self.assertEqual(100, self.loader.get_progress())
 
     def test_load_from_client(self):
         self.source_client = isc.datasrc.DataSourceClient('sqlite3',
@@ -130,16 +128,13 @@ class ZoneLoaderTests(unittest.TestCase):
                                              self.source_client)
 
         self.assertEqual(0, self.loader.get_rr_count())
-        self.assertEqual(0, self.loader.get_size())
-        self.assertEqual(0, self.loader.get_position())
+        self.assertEqual(PROGRESS_UNKNOWN, self.loader.get_progress())
 
         self.check_load()
 
-        # In case of loading from another data source, size and position are
-        # always 0.
+        # In case of loading from another data source, progress is unknown.
         self.assertEqual(8, self.loader.get_rr_count())
-        self.assertEqual(0, self.loader.get_size())
-        self.assertEqual(0, self.loader.get_position())
+        self.assertEqual(PROGRESS_UNKNOWN, self.loader.get_progress())
 
     def check_load_incremental(self, from_file=True):
         # New zone has 8 RRs
@@ -148,11 +143,11 @@ class ZoneLoaderTests(unittest.TestCase):
         # New zone should not have been loaded yet
         self.check_zone_soa(ORIG_SOA_TXT)
 
-        # In case it's from a zone file, check get_size() and get_position()
-        # are different.  expected values are taken from the test zone file.
+        # In case it's from a zone file, get_progress should be in the middle
+        # of (0, 100).  expected value is taken from the test zone file
+        # (total size = 422, current position = 288)
         if from_file:
-            self.assertEqual(422, self.loader.get_size())
-            self.assertEqual(288, self.loader.get_position())
+            self.assertEqual(int(288 * 100 / 422), self.loader.get_progress())
 
         # After 5 more, it should return True (only having read 3)
         self.assertTrue(self.loader.load_incremental(5))

src/lib/python/isc/datasrc/zone_loader_inc.cc

@@ -129,63 +129,43 @@ Exceptions:\n\
 \n\
 ";
 
-const char* const ZoneLoader_getSize_doc = "\
-get_size() -> integer\n\
-\n\
-Return the (estimated) total size of the entire zone.\n\
-\n\
-This method returns some hint on how large the zone will be when\n\
-completing the load. The returned size is a conceptual value that can\n\
-internally mean anything. The intended usage of the value is to\n\
-compare it to the return value of get_position() to estimate the\n\
-progress of the load at the time of the call.\n\
+const char* const ZoneLoader_getProgress_doc = "\
+get_progress() -> int\n\
+\n\
+Return the current progress of the loader in percentage.\n\
+\n\
+This method returns the current estimated progress of loader in\n\
+percentage; it's 0 before starting the load, and 100 at the\n\
+completion, and a value between 0 and 100 in the middle of loading.\n\
+It's an implementation detail how to calculate the progress, which may\n\
+vary depending on how the loader is constructed and may even be\n\
+impossible to detect effectively.\n\
+\n\
+If the progress cannot be determined, this method returns a special\n\
+value of PROGRESS_UNKNOWN, which is not included in the range between\n\
+0 and 100.\n\
+\n\
+As such, the application should use the return value only for\n\
+informational purposes such as logging. For example, it shouldn't be\n\
+used to determine whether loading is completed by comparing it to 100.\n\
+It should also expect the possibility of getting PROGRESS_UNKNOWN at\n\
+any call to this method; it shouldn't assume the specific way of\n\
+internal implementation as described below (which is provided for\n\
+informational purposes only).\n\
 \n\
 In this implementation, if the loader is constructed with a file name,\n\
-the returned size is the size of the zone file. If it includes other\n\
-files via the $INCLUDE directive, it will be the sum of the file sizes\n\
-of all such files that the loader has handled. Note that it may be\n\
-smaller than the final size if there are more files to be included\n\
-which the loader has not seen by the time of the call.\n\
-\n\
-Currently, if the loader is constructed with another data source\n\
-client, this method always returns 0. In future, it may be possible to\n\
-return something more effective, e.g, the total number of RRs if the\n\
-underlying data source can provide that information efficiently.\n\
-\n\
-In any case, the caller shouldn't assume anything specific about the\n\
-meaning of the value other than for comparing it to the result of\n\
-get_position().\n\
-\n\
-Exceptions:\n\
-  None\n\
-\n\
-";
-
-const char* const ZoneLoader_getPosition_doc = "\
-get_position() -> integer\n\
-\n\
-Return the current position of the loader in the zone being loaded.\n\
-\n\
-This method returns a conceptual \"position\" of this loader in the\n\
-loader relative to the return value of get_size(). Before starting the\n\
-load the position is set to 0; on successful completion, it will be\n\
-equal to the get_size() value; in the middle of the load, it's\n\
-expected to be between these values, which would give some hint about\n\
-the progress of the loader.\n\
-\n\
-In the current implementation, if the loader is constructed with a\n\
-file name, the returned value is the number of characters from the\n\
-zone file (and any included files) recognized by the underlying zone\n\
-file parser.\n\
-\n\
-If it's constructed with another data source client, it's always 0 for\n\
-now; however, if get_position() is extended in this case as documented\n\
-(see the method description), the result of get_rr_count() could be\n\
-used for the current position.\n\
-\n\
-Like get_size(), the value is conceptual and the caller shouldn't\n\
-assume any specific meaning of the value except for comparing it to\n\
-get_size() results.\n\
+the progress value is measured by the number of characters read from\n\
+the zone file divided by the size of the zone file (with taking into\n\
+account any included files). Note that due to the possibility of\n\
+intermediate included files, the total file size cannot be fully fixed\n\
+until the completion of the load. And, due to this possibility, return\n\
+values from this method may not always increase monotonically.\n\
+\n\
+If it's constructed with another data source client, this method\n\
+always returns PROGRESS_UNKNOWN; in future, however, it may become\n\
+possible to return something more useful, e.g, based on the result of\n\
+get_rr_count() and the total number of RRs if the underlying data\n\
+source can provide the latter value efficiently.\n\
 \n\
 Exceptions:\n\
   None\n\

src/lib/python/isc/datasrc/zone_loader_python.cc

@@ -194,15 +194,9 @@ ZoneLoader_getRRCount(PyObject* po_self, PyObject*) {
 }
 
 PyObject*
-ZoneLoader_getSize(PyObject* po_self, PyObject*) {
+ZoneLoader_getProgress(PyObject* po_self, PyObject*) {
     s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
-    return (Py_BuildValue("I", self->cppobj->getSize()));
-}
-
-PyObject*
-ZoneLoader_getPosition(PyObject* po_self, PyObject*) {
-    s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
-    return (Py_BuildValue("I", self->cppobj->getPosition()));
+    return (Py_BuildValue("i", self->cppobj->getProgress()));
 }
 
 // This list contains the actual set of functions we have in
@@ -217,9 +211,8 @@ PyMethodDef ZoneLoader_methods[] = {
       ZoneLoader_loadIncremental_doc },
     { "get_rr_count", ZoneLoader_getRRCount, METH_NOARGS,
       ZoneLoader_getRRCount_doc },
-    { "get_size", ZoneLoader_getSize, METH_NOARGS, ZoneLoader_getSize_doc },
-    { "get_position", ZoneLoader_getPosition, METH_NOARGS,
-      ZoneLoader_getPosition_doc },
+    { "get_progress", ZoneLoader_getProgress, METH_NOARGS,
+      ZoneLoader_getProgress_doc },
     { NULL, NULL, 0, NULL }
 };