BIND 10 trac2573, updated. 2992f269ed62c67a5fb60ad60c0f7dcd89fba957 [2573] count added RR within datasrc loader, and deprecate MasterLoader method
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 16 19:39:08 UTC 2013
The branch, trac2573 has been updated
via 2992f269ed62c67a5fb60ad60c0f7dcd89fba957 (commit)
via 2f061fe969a7af07cecd9cd5d166f5b2d5b327aa (commit)
via 6fbf5830da2a9ed728a865715bab0d59e1a5431b (commit)
from ae4b62b7f869cc7e66ef691ab20faff93566ac5b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 2992f269ed62c67a5fb60ad60c0f7dcd89fba957
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed Jan 16 11:37:16 2013 -0800
[2573] count added RR within datasrc loader, and deprecate MasterLoader method
in fact, the master loader's method is redundant because its user can count
them the same way. its own method might be of some use in the future,
but until then I'd simpy remove it based on YAGNI.
Also made some unrelated cleanup based on recent clarification of
null tests on pointer/shared pointers.
commit 2f061fe969a7af07cecd9cd5d166f5b2d5b327aa
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed Jan 16 10:48:26 2013 -0800
[2573] updated python wrapper using loader's getProgress.
get_size/get_position were deprecated and removed.
commit 6fbf5830da2a9ed728a865715bab0d59e1a5431b
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed Jan 16 10:29:02 2013 -0800
[2573] introduce getProgress instead size/position.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/tests/zone_loader_unittest.cc | 57 +++++--------
src/lib/datasrc/zone_loader.cc | 59 ++++++++++---
src/lib/datasrc/zone_loader.h | 86 +++++++++----------
src/lib/dns/master_loader.cc | 5 --
src/lib/dns/master_loader.h | 10 ---
src/lib/dns/tests/master_loader_unittest.cc | 12 +--
src/lib/python/isc/datasrc/datasrc.cc | 19 +++++
.../python/isc/datasrc/tests/zone_loader_test.py | 27 +++---
src/lib/python/isc/datasrc/zone_loader_inc.cc | 90 ++++++++------------
src/lib/python/isc/datasrc/zone_loader_python.cc | 15 +---
10 files changed, 176 insertions(+), 204 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/tests/zone_loader_unittest.cc b/src/lib/datasrc/tests/zone_loader_unittest.cc
index 43d5e53..9c7eafd 100644
--- a/src/lib/datasrc/tests/zone_loader_unittest.cc
+++ b/src/lib/datasrc/tests/zone_loader_unittest.cc
@@ -180,17 +180,16 @@ protected:
};
// Use the loader to load an unsigned zone.
-TEST_F(ZoneLoaderTest, copyUnsigned) {
+TEST_F(ZoneLoaderTest , copyUnsigned) {
prepareSource(Name::ROOT_NAME(), "root.zone");
ZoneLoader loader(destination_client_, Name::ROOT_NAME(), source_client_);
// It gets the updater directly in the constructor
ASSERT_EQ(1, destination_client_.provided_updaters_.size());
EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
- // Counters are initialized to 0.
+ // Counter is initialized to 0, progress is "unknown" in case of copy.
EXPECT_EQ(0, loader.getRRCount());
- EXPECT_EQ(0, loader.getSize());
- EXPECT_EQ(0, loader.getPosition());
+ EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
// Now load the whole zone
loader.load();
@@ -202,10 +201,9 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
EXPECT_EQ(34, destination_client_.rrsets_.size());
// Check various counters. getRRCount should be identical of the RRs
- // we've seen. size and position should be still 0 in the copy operation.
+ // we've seen. progress is still "unknown" in the copy operation.
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(0, loader.getSize());
- EXPECT_EQ(0, loader.getPosition());
+ EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
// Ensure known order.
std::sort(destination_client_.rrsets_.begin(),
@@ -234,11 +232,10 @@ TEST_F(ZoneLoaderTest, copyUnsignedIncremental) {
// Not committed yet, we didn't complete the loading
EXPECT_FALSE(destination_client_.commit_called_);
- // Check we can get intermediate counters. size/position are always 0
+ // Check we can get intermediate counters. progress is always "unknown"
// in case of copy.
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(0, loader.getSize());
- EXPECT_EQ(0, loader.getPosition());
+ EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
// This is unusual, but allowed. Check it doesn't do anything
loader.loadIncremental(0);
@@ -309,10 +306,9 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
TEST_DATA_DIR "/root.zone");
- // Counters are initialized to 0.
+ // Counter and progress are initialized to 0.
EXPECT_EQ(0, loader.getRRCount());
- EXPECT_EQ(0, loader.getSize());
- EXPECT_EQ(0, loader.getPosition());
+ EXPECT_EQ(0, loader.getProgress());
// It gets the updater directly in the constructor
ASSERT_EQ(1, destination_client_.provided_updaters_.size());
@@ -326,12 +322,10 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
// The count is 34 because we expect the RRs to be separated.
EXPECT_EQ(34, destination_client_.rrsets_.size());
- // Check various counters. getRRCount should be identical of the RRs
- // we've seen. size and position should be identical of the RRs
- // file (hardcoded, assuming we won't change it so often).
+ // getRRCount should be identical of the RRs we've seen. progress
+ // should reach 100%.
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(1541, loader.getSize());
- EXPECT_EQ(1541, loader.getPosition());
+ EXPECT_EQ(100, loader.getProgress());
// Ensure known order.
std::sort(destination_client_.rrsets_.begin(),
@@ -341,13 +335,6 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
EXPECT_EQ("m.root-servers.net. 3600000 IN AAAA 2001:dc3::35\n",
destination_client_.rrsets_.back());
- // Check various counters. getRRCount should be identical of the RRs
- // we've seen. size and position should be equal to the size of the zone
- // file (hardcoded assuming we won't change it so often).
- EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(1541, loader.getSize());
- EXPECT_EQ(1541, loader.getPosition());
-
// It isn't possible to try again now
EXPECT_THROW(loader.load(), isc::InvalidOperation);
EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);
@@ -362,8 +349,7 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
// Counters are initialized to 0.
EXPECT_EQ(0, loader.getRRCount());
- EXPECT_EQ(0, loader.getSize());
- EXPECT_EQ(0, loader.getPosition());
+ EXPECT_EQ(0, loader.getProgress());
// Try loading few RRs first.
loader.loadIncremental(10);
@@ -375,22 +361,23 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
EXPECT_EQ(10, destination_client_.rrsets_.size());
EXPECT_FALSE(destination_client_.commit_called_);
- // Check we can get intermediate counters. size is the size of the
- // zone file; position is the offset to the end of 10th RR (both
- // hardcoded).
+ // Check we can get intermediate counters. expected progress is calculated
+ // based on the size of the zone file and the offset to the end of 10th RR
+ // (subject to future changes to the file, but we assume it's a rare
+ // event.). This also involves floating point operation, which may
+ // cause a margin error depending on environments. If it happens we
+ // should loosen the test condition to accept some margin.
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(1541, loader.getSize());
- EXPECT_EQ(428, loader.getPosition());
+ EXPECT_EQ(27, loader.getProgress()); // file size = 1541, offset = 428
// We can finish the rest
loader.loadIncremental(30);
EXPECT_EQ(34, destination_client_.rrsets_.size());
EXPECT_TRUE(destination_client_.commit_called_);
- // Counters are updated accordingly. size and position are now identical.
+ // Counters are updated accordingly. progress should reach 100%.
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(1541, loader.getSize());
- EXPECT_EQ(1541, loader.getPosition());
+ EXPECT_EQ(100, loader.getProgress());
// No more loading now
EXPECT_THROW(loader.load(), isc::InvalidOperation);
diff --git a/src/lib/datasrc/zone_loader.cc b/src/lib/datasrc/zone_loader.cc
index c74ad3b..6e22ed2 100644
--- a/src/lib/datasrc/zone_loader.cc
+++ b/src/lib/datasrc/zone_loader.cc
@@ -21,14 +21,22 @@
#include <datasrc/zone.h>
#include <dns/rrset.h>
+#include <dns/name.h>
+#include <dns/master_loader.h>
+#include <dns/master_lexer.h>
+
+#include <boost/bind.hpp>
using isc::dns::Name;
using isc::dns::ConstRRsetPtr;
using isc::dns::MasterLoader;
+using isc::dns::MasterLexer;
namespace isc {
namespace datasrc {
+const int ZoneLoader::PROGRESS_UNKNOWN = -1;
+
ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
DataSourceClient& source) :
// Separate the RRsets as that is possibly faster (the data source doesn't
@@ -56,6 +64,21 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
}
}
+namespace {
+// Unified callback to install RR and increment RR count at the same time.
+void
+addRR(const dns::Name& name, const dns::RRClass& rrclass,
+ const dns::RRType& type, const dns::RRTTL& ttl,
+ const dns::rdata::RdataPtr& data, ZoneUpdater* updater,
+ size_t* rr_count)
+{
+ isc::dns::BasicRRset rrset(name, rrclass, type, ttl);
+ rrset.addRdata(data);
+ updater->addRRset(rrset);
+ ++*rr_count;
+}
+}
+
ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
const char* filename) :
updater_(destination.getUpdater(zone_name, true, false)),
@@ -73,7 +96,8 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
createMasterLoaderCallbacks(zone_name,
updater_->getFinder().getClass(),
&loaded_ok_),
- createMasterLoaderAddCallback(*updater_)));
+ boost::bind(addRR, _1, _2, _3, _4, _5,
+ updater_.get(), &rr_count_)));
}
}
@@ -108,8 +132,8 @@ ZoneLoader::loadIncremental(size_t limit) {
"Loading has been completed previously");
}
- if (iterator_ == ZoneIteratorPtr()) {
- assert(loader_.get() != NULL);
+ if (!iterator_) {
+ assert(loader_);
try {
complete_ = loader_->loadIncremental(limit);
} catch (const isc::dns::MasterLoaderError& e) {
@@ -118,7 +142,6 @@ ZoneLoader::loadIncremental(size_t limit) {
if (complete_ && !loaded_ok_) {
isc_throw(MasterFileError, "Error while loading master file");
}
- rr_count_ = loader_->getRRCount();
} else {
complete_ = copyRRsets(updater_, iterator_, limit, rr_count_);
}
@@ -134,20 +157,30 @@ ZoneLoader::getRRCount() const {
return (rr_count_);
}
-size_t
-ZoneLoader::getSize() const {
+int
+ZoneLoader::getProgress() const {
if (!loader_) {
- return (0);
+ return (PROGRESS_UNKNOWN);
}
- return (loader_->getSize());
-}
-size_t
-ZoneLoader::getPosition() const {
- if (!loader_) {
+ const size_t pos = loader_->getPosition();
+ const size_t total_size = loader_->getSize();
+
+ // If the current position is 0, progress should definitely be 0; we
+ // don't bother to check the total size even if it's "unknown".
+ if (pos == 0) {
return (0);
}
- return (loader_->getPosition());
+
+ // These cases shouldn't happen with our usage of MasterLoader. So, in
+ // theory, we could throw here; however, since this method is expected
+ // to be used for informational purposes only, that's probably too harsh.
+ // So we return "unknown" instead.
+ if (total_size == MasterLexer::SOURCE_SIZE_UNKNOWN || total_size == 0) {
+ return (PROGRESS_UNKNOWN);
+ }
+
+ return ((static_cast<double>(pos) / total_size) * 100);
}
} // end namespace datasrc
diff --git a/src/lib/datasrc/zone_loader.h b/src/lib/datasrc/zone_loader.h
index fb787b6..a60ae6b 100644
--- a/src/lib/datasrc/zone_loader.h
+++ b/src/lib/datasrc/zone_loader.h
@@ -150,60 +150,50 @@ public:
/// \throw None
size_t getRRCount() const;
- /// \brief Return the (estimated) total size of the entire zone.
- ///
- /// This method returns some hint on how large the zone will be when
- /// completing the load. The returned size is a conceptual value that
- /// can internally mean anything. The intended usage of the value is
- /// to compare it to the return value of \c getPosition() to estimate
- /// the progress of the load at the time of the call.
+ /// \brief Return the current progress of the loader in percentage.
+ ///
+ /// This method returns the current estimated progress of loader in
+ /// percentage; it's 0 before starting the load, and 100 at the
+ /// completion, and a value between 0 and 100 in the middle of loading.
+ /// It's an implementation detail how to calculate the progress, which
+ /// may vary depending on how the loader is constructed and may even be
+ /// impossible to detect effectively.
+ ///
+ /// If the progress cannot be determined, this method returns a special
+ /// value of PROGRESS_UNKNOWN, which is not included in the range between
+ /// 0 and 100.
+ ///
+ /// As such, the application should use the return value only for
+ /// informational purposes such as logging. For example, it shouldn't
+ /// be used to determine whether loading is completed by comparing it
+ /// to 100. It should also expect the possibility of getting
+ /// \c PROGRESS_UNKNOWN at any call to this method; it shouldn't assume
+ /// the specific way of internal implementation as described below (which
+ /// is provided for informational purposes only).
///
/// In this implementation, if the loader is constructed with a file
- /// name, the returned size is the size of the zone file. If it includes
- /// other files via the $INCLUDE directive, it will be the sum of the
- /// file sizes of all such files that the loader has handled.
- /// Note that it may be smaller than the final size if there are more
- /// files to be included which the loader has not seen by the time of
- /// the call.
- ///
- /// Currently, if the loader is constructed with another data source
- /// client, this method always returns 0. In future, it may be possible
- /// to return something more effective, e.g, the total number of RRs
- /// if the underlying data source can provide that information efficiently.
- ///
- /// In any case, the caller shouldn't assume anything specific about the
- /// meaning of the value other than for comparing it to the result of
- /// \c getPosition().
+ /// name, the progress value is measured by the number of characters
+ /// read from the zone file divided by the size of the zone file
+ /// (with taking into account any included files). Note that due to
+ /// the possibility of intermediate included files, the total file size
+ /// cannot be fully fixed until the completion of the load. And, due to
+ /// this possibility, return values from this method may not always
+ /// increase monotonically.
+ ///
+ /// If it's constructed with another data source client, this method
+ /// always returns \c PROGRESS_UNKNOWN; in future, however, it may become
+ /// possible to return something more useful, e.g, based on the result
+ /// of \c getRRCount() and the total number of RRs if the underlying data
+ /// source can provide the latter value efficiently.
///
/// \throw None
- size_t getSize() const;
+ int getProgress() const;
- /// \brief Return the current position of the loader in the zone being
- /// loaded.
- ///
- /// This method returns a conceptual "position" of this loader in the
- /// loader relative to the return value of \c getSize(). Before starting
- /// the load the position is set to 0; on successful completion,
- /// it will be equal to the \c getSize() value; in the middle of the load,
- /// it's expected to be between these values, which would give some
- /// hint about the progress of the loader.
- ///
- /// In the current implementation, if the loader is constructed with a
- /// file name, the returned value is the number of characters from the
- /// zone file (and any included files) recognized by the underlying zone
- /// file parser.
+ /// \brief A special value for \c getProgress, meaning the progress is
+ /// unknown.
///
- /// If it's constructed with another data source client, it's always
- /// 0 for now; however, if \c getPosition() is extended in this case
- /// as documented (see the method description), the result of
- /// \c getRRCount() could be used for the current position.
- ///
- /// Like \c getSize(), the value is conceptual and the caller shouldn't
- /// assume any specific meaning of the value except for comparing it
- /// to \c getSize() results.
- ///
- /// \throw None
- size_t getPosition() const;
+ /// See the method description for details.
+ static const int PROGRESS_UNKNOWN;
private:
/// \brief The iterator used as source of data in case of the copy mode.
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index b5bf1f0..29e7e1d 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -638,11 +638,6 @@ MasterLoader::loadedSucessfully() const {
}
size_t
-MasterLoader::getRRCount() const {
- return (impl_->rr_count_);
-}
-
-size_t
MasterLoader::getSize() const {
return (impl_->getSize());
}
diff --git a/src/lib/dns/master_loader.h b/src/lib/dns/master_loader.h
index 0c089fb..37b2ef4 100644
--- a/src/lib/dns/master_loader.h
+++ b/src/lib/dns/master_loader.h
@@ -142,16 +142,6 @@ public:
/// finishing the load.
bool loadedSucessfully() const;
- /// \brief Return the number of RRs loaded.
- ///
- /// This method returns the number of RRs loaded via this loader by the
- /// time of the call. Before starting the load it will return 0.
- /// It will return the total number of RRs of the zone on and after
- /// completing the load.
- ///
- /// \throw None
- size_t getRRCount() const;
-
/// \brief Return the total size of the zone files and streams.
///
/// This method returns the size of the source of the zone to be loaded
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 0c6202f..636f73d 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -156,7 +156,6 @@ TEST_F(MasterLoaderTest, basicLoad) {
// The following three should be set to 0 initially in case the loader
// is constructed from a file name.
- EXPECT_EQ(0, loader_->getRRCount());
EXPECT_EQ(0, loader_->getSize());
EXPECT_EQ(0, loader_->getPosition());
@@ -168,7 +167,6 @@ TEST_F(MasterLoaderTest, basicLoad) {
// Hardcode expected values taken from the test data file, assuming it
// won't change too often.
- EXPECT_EQ(4, loader_->getRRCount());
EXPECT_EQ(549, loader_->getSize());
EXPECT_EQ(549, loader_->getPosition());
@@ -323,7 +321,6 @@ TEST_F(MasterLoaderTest, popAfterError) {
EXPECT_FALSE(loader_->loadedSucessfully());
EXPECT_EQ(1, errors_.size()); // For the broken RR
EXPECT_EQ(1, warnings_.size()); // For missing EOLN
- EXPECT_EQ(1, loader_->getRRCount()); // broken RR shouldn't be counted
// The included file doesn't contain anything usable, but the
// line after the include should be there.
@@ -342,7 +339,6 @@ TEST_F(MasterLoaderTest, streamConstructor) {
// Unlike the basicLoad test, if we construct the loader from a stream
// getSize() returns the data size in the stream immediately after the
// construction.
- EXPECT_EQ(0, loader_->getRRCount());
EXPECT_EQ(zone_data.size(), loader_->getSize());
EXPECT_EQ(0, loader_->getPosition());
@@ -356,9 +352,7 @@ TEST_F(MasterLoaderTest, streamConstructor) {
checkRR("correct.example.org", RRType::A(), "192.0.2.2");
// On completion of the load, both getSize() and getPosition() return the
- // size of the data, and getRRCount() returns the number of loaded RRs,
- // which is 2 in this case.
- EXPECT_EQ(2, loader_->getRRCount());
+ // size of the data.
EXPECT_EQ(zone_data.size(), loader_->getSize());
EXPECT_EQ(zone_data.size(), loader_->getPosition());
}
@@ -368,10 +362,8 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
RRClass::IN(), MasterLoader::MANY_ERRORS);
- EXPECT_EQ(0, loader_->getRRCount()); // initial count should be 2
EXPECT_FALSE(loader_->loadedSucessfully());
EXPECT_FALSE(loader_->loadIncremental(2));
- EXPECT_EQ(2, loader_->getRRCount()); // number of loaded RR so far
EXPECT_FALSE(loader_->loadedSucessfully());
EXPECT_TRUE(errors_.empty());
@@ -394,8 +386,6 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
checkRR("www.example.org", RRType::A(), "192.0.2.1");
checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
-
- EXPECT_EQ(4, loader_->getRRCount()); // on completion it's # of zone's RRs.
}
// Try loading from file that doesn't exist. There should be single error
diff --git a/src/lib/python/isc/datasrc/datasrc.cc b/src/lib/python/isc/datasrc/datasrc.cc
index 37c9baa..f737b3b 100644
--- a/src/lib/python/isc/datasrc/datasrc.cc
+++ b/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);
}
diff --git a/src/lib/python/isc/datasrc/tests/zone_loader_test.py b/src/lib/python/isc/datasrc/tests/zone_loader_test.py
index 57ac49a..8caef78 100644
--- a/src/lib/python/isc/datasrc/tests/zone_loader_test.py
+++ b/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))
diff --git a/src/lib/python/isc/datasrc/zone_loader_inc.cc b/src/lib/python/isc/datasrc/zone_loader_inc.cc
index 7fa994a..d9bb120 100644
--- a/src/lib/python/isc/datasrc/zone_loader_inc.cc
+++ b/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\
diff --git a/src/lib/python/isc/datasrc/zone_loader_python.cc b/src/lib/python/isc/datasrc/zone_loader_python.cc
index 47c7ddf..668184d 100644
--- a/src/lib/python/isc/datasrc/zone_loader_python.cc
+++ b/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 }
};
More information about the bind10-changes
mailing list