From e313cf6406bf734ce2c8aed16a2b632ebf749d37 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Mon, 14 May 2012 21:57:43 -0700 Subject: [PATCH] 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. --- src/allmydata/check_results.py | 10 +++++----- src/allmydata/immutable/filenode.py | 13 +++++++------ src/allmydata/test/test_checker.py | 2 +- src/allmydata/test/test_repairer.py | 4 ++-- 4 files changed, 15 insertions(+), 14 deletions(-) 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 -- 2.37.2