From: Brian Warner <warner@lothar.com>
Date: Tue, 15 May 2012 04:57:43 +0000 (-0700)
Subject: CheckResults: start hiding .data, first step to clean it up
X-Git-Url: https://git.rkrishnan.org/%5B/%5D%20/file/reliability?a=commitdiff_plain;h=e313cf6406bf734ce2c8aed16a2b632ebf749d37;p=tahoe-lafs%2Ftahoe-lafs.git

CheckResults: start hiding .data, first step to clean it up

The goal is to make CheckResults more strongly typed, and remove the
ambiguous ".data" field in favor of a bunch of specific counters and
sharelists, so I can changes .sharemap and .servermap to use IServer
instances instead of string serverids. By cleaning this up first, I hope
to get that task done with less debugging.
---

diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py
index 7cb79705..7224a6f8 100644
--- a/src/allmydata/check_results.py
+++ b/src/allmydata/check_results.py
@@ -12,9 +12,9 @@ class CheckResults:
         self.uri = uri
         self.storage_index = storage_index
         self.problems = []
-        self.data = {"count-corrupt-shares": 0,
-                     "list-corrupt-shares": [],
-                     }
+        self._data = {"count-corrupt-shares": 0,
+                      "list-corrupt-shares": [],
+                      }
         self.summary = ""
         self.report = []
 
@@ -34,7 +34,7 @@ class CheckResults:
     def set_needs_rebalancing(self, needs_rebalancing):
         self.needs_rebalancing_p = bool(needs_rebalancing)
     def set_data(self, data):
-        self.data.update(data)
+        self._data.update(data)
     def set_summary(self, summary):
         assert isinstance(summary, str) # should be a single string
         self.summary = summary
@@ -62,7 +62,7 @@ class CheckResults:
     def needs_rebalancing(self):
         return self.needs_rebalancing_p
     def get_data(self):
-        return self.data
+        return self._data
 
     def get_summary(self):
         return self.summary
diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index c6f31f0a..dbd09e0b 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -126,20 +126,21 @@ class CiphertextFileNode:
         # clone the cr (check results) to form the basis of the
         # prr (post-repair results)
         prr = CheckResults(cr.uri, cr.storage_index)
-        prr.data = copy.deepcopy(cr.data)
+        prr_data = copy.deepcopy(cr.get_data())
 
-        servers_responding = set(prr.data['servers-responding'])
-        sm = prr.data['sharemap']
+        servers_responding = set(prr_data['servers-responding'])
+        sm = prr_data['sharemap']
         assert isinstance(sm, DictOfSets), sm
         for shnum, servers in ur.get_sharemap().items():
             for s in servers:
                 sm.add(shnum, s.get_serverid())
                 servers_responding.add(s.get_serverid())
         servers_responding = sorted(servers_responding)
-        prr.data['servers-responding'] = servers_responding
-        prr.data['count-shares-good'] = len(sm)
+        prr_data['servers-responding'] = servers_responding
+        prr_data['count-shares-good'] = len(sm)
         good_hosts = len(reduce(set.union, sm.itervalues(), set()))
-        prr.data['count-good-share-hosts'] = good_hosts
+        prr_data['count-good-share-hosts'] = good_hosts
+        prr.set_data(prr_data)
         verifycap = self._verifycap
         is_healthy = bool(len(sm) >= verifycap.total_shares)
         is_recoverable = bool(len(sm) >= verifycap.needed_shares)
diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py
index f88ba5c2..dc7d90fd 100644
--- a/src/allmydata/test/test_checker.py
+++ b/src/allmydata/test/test_checker.py
@@ -366,7 +366,7 @@ class BalancingAct(GridTestMixin, unittest.TestCase):
         def _check_and_repair(_):
             return self.imm.check_and_repair(Monitor())
         def _check_counts(crr, shares_good, good_share_hosts):
-            p_crr = crr.get_post_repair_results().data
+            p_crr = crr.get_post_repair_results().get_data()
             #print self._pretty_shares_chart(self.uri)
             self.failUnlessEqual(p_crr['count-shares-good'], shares_good)
             self.failUnlessEqual(p_crr['count-good-share-hosts'],
diff --git a/src/allmydata/test/test_repairer.py b/src/allmydata/test/test_repairer.py
index 65d2b2c2..531db0d1 100644
--- a/src/allmydata/test/test_repairer.py
+++ b/src/allmydata/test/test_repairer.py
@@ -498,7 +498,7 @@ class Repairer(GridTestMixin, unittest.TestCase, RepairTestMixin,
             self.failIfBigger(delta_reads, MAX_DELTA_READS)
             self.failIfBigger(delta_allocates, (DELTA_WRITES_PER_SHARE * 7))
             self.failIf(pre.is_healthy())
-            self.failUnless(post.is_healthy(), post.data)
+            self.failUnless(post.is_healthy(), post.get_data())
 
             # Make sure we really have 10 shares.
             shares = self.find_uri_shares(self.uri)
@@ -724,7 +724,7 @@ class Repairer(GridTestMixin, unittest.TestCase, RepairTestMixin,
             # not respond to the pre-repair filecheck
             prr = rr.get_post_repair_results()
             expected = set(self.g.get_all_serverids())
-            self.failUnlessEqual(expected, set(prr.data["servers-responding"]))
+            self.failUnlessEqual(expected, set(prr.get_data()["servers-responding"]))
         d.addCallback(_check)
         return d