Make the cloud backend report corrupted shares correctly. fixes #1566

Signed-off-by: Daira Hopwood <david-sarah@jacaranda.org>

diff --git a/src/allmydata/storage/backends/cloud/cloud_backend.py b/src/allmydata/storage/backends/cloud/cloud_backend.py
index efe4e6277efb80014a9fdb4181b237760093af10..247aa2287c2b0f50730b5c08b46715116a9b3225 100644 (file)
--- a/src/allmydata/storage/backends/cloud/cloud_backend.py
+++ b/src/allmydata/storage/backends/cloud/cloud_backend.py
@@ -7,11 +7,10 @@ from zope.interface import implements
 from allmydata.interfaces import IStorageBackend, IShareSet
 
 from allmydata.node import InvalidValueError
-from allmydata.util.deferredutil import gatherResults
 from allmydata.util.assertutil import _assert
 from allmydata.util.dictutil import NumDict
 from allmydata.util.encodingutil import quote_output
-from allmydata.storage.common import si_a2b, NUM_RE
+from allmydata.storage.common import si_a2b, NUM_RE, CorruptStoredShareError
 from allmydata.storage.bucket import BucketWriter
 from allmydata.storage.backends.base import Backend, ShareSet
 from allmydata.storage.backends.cloud.immutable import ImmutableCloudShareForReading, ImmutableCloudShareForWriting
@@ -129,11 +128,18 @@ class CloudShareSet(ShareSet):
                         # If they don't, that will cause an error on reading.
                         shnum_to_total_size.add_num(int(shnumstr), int(item.size))
 
-            return gatherResults([get_cloud_share(self._container, si, shnum, total_size)
-                                  for (shnum, total_size) in shnum_to_total_size.items_sorted_by_key()])
+            return defer.DeferredList([get_cloud_share(self._container, si, shnum, total_size)
+                                       for (shnum, total_size) in shnum_to_total_size.items_sorted_by_key()],
+                                      consumeErrors=True)
         d.addCallback(_get_shares)
-        # TODO: return information about corrupt shares.
-        d.addCallback(lambda shares: (shares, set()) )
+        def _got_list(outcomes):
+            # DeferredList gives us a list of (success, result) pairs, which we
+            # convert to a pair (list of shares, set of corrupt shnums).
+            shares = [share for (success, share) in outcomes if success]
+            corrupted = set([f.value.shnum for (success, f) in outcomes
+                             if not success and isinstance(f.value, CorruptStoredShareError)])
+            return (shares, corrupted)
+        d.addCallback(_got_list)
         return d
 
     def _locked_get_share(self, shnum):

diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index d0d4d87a41ea428f8c3e5dd467ee5010f3da63e5..8973c5aab62495242b970b86ee743e1432f3d407 100644 (file)
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -2751,10 +2751,6 @@ class ServerWithMockCloudBackend(WithMockCloudBackend, ServerTest, unittest.Test
         # This causes ContainerListMixin to be exercised.
         self.patch(mock_cloud, 'MAX_KEYS', 2)
 
-    def test_bad_container_version(self):
-        return ServerTest.test_bad_container_version(self)
-    test_bad_container_version.todo = "The cloud backend hasn't been modified to fix ticket #1566."
-
 
     def _describe_level(self, level):
         return getattr(LogEvent, 'LEVELMAP', {}).get(level, str(level))
@@ -3133,10 +3129,6 @@ class MutableServerWithMockCloudBackend(WithMockCloudBackend, MutableServerTest,
         # This causes ContainerListMixin to be exercised.
         self.patch(mock_cloud, 'MAX_KEYS', 2)
 
-    def test_bad_magic(self):
-        return MutableServerTest.test_bad_magic(self)
-    test_bad_magic.todo = "The cloud backend hasn't been modified to fix ticket #1566."
-
 
 class MutableServerWithDiskBackend(WithDiskBackend, MutableServerTest, unittest.TestCase):
     # There are no mutable tests specific to a disk backend.