2752: Remove unused CollectionWriter checkpoint hook.

diff --git a/sdk/python/arvados/collection.py b/sdk/python/arvados/collection.py
index f339381ba1a95a34b55fa402fa882d1e9a88386b..29c44b416da0e1a9b30ffbc4dfc6c945912c0595 100644 (file)
--- a/sdk/python/arvados/collection.py
+++ b/sdk/python/arvados/collection.py
@@ -194,11 +194,6 @@ class CollectionWriter(object):
                 self._work_trees()
             else:
                 break
-            self.checkpoint_state()
-
-    def checkpoint_state(self):
-        # Subclasses can implement this method to, e.g., report or record state.
-        pass
 
     def _work_file(self):
         while True:
@@ -280,7 +275,6 @@ class CollectionWriter(object):
             self._current_stream_locators += [Keep.put(data_buffer[0:self.KEEP_BLOCK_SIZE])]
             self._data_buffer = [data_buffer[self.KEEP_BLOCK_SIZE:]]
             self._data_buffer_len = len(self._data_buffer[0])
-            self.checkpoint_state()
 
     def start_new_file(self, newfilename=None):
         self.finish_current_file()

diff --git a/sdk/python/tests/test_collections.py b/sdk/python/tests/test_collections.py
index 4d1e1505d6e8e33835c73b12f27b8bea7a06b12e..d867f7ba936e519e7a976e76945df12caa331001 100644 (file)
--- a/sdk/python/tests/test_collections.py
+++ b/sdk/python/tests/test_collections.py
@@ -16,16 +16,8 @@ from arvados_testutil import ArvadosKeepLocalStoreTestCase
 class TestResumableWriter(arvados.ResumableCollectionWriter):
     KEEP_BLOCK_SIZE = 1024  # PUT to Keep every 1K.
 
-    def __init__(self):
-        self.saved_states = []
-        return super(TestResumableWriter, self).__init__()
-
-    def checkpoint_state(self):
-        self.saved_states.append(self.dump_state(copy.deepcopy))
-
-    def last_state(self):
-        assert self.saved_states, "resumable writer did not save any state"
-        return self.saved_states[-1]
+    def current_state(self):
+        return self.dump_state(copy.deepcopy)
 
 
 class ArvadosCollectionsTest(ArvadosKeepLocalStoreTestCase):
@@ -525,27 +517,11 @@ class ArvadosCollectionsTest(ArvadosKeepLocalStoreTestCase):
             cwriter.manifest_text(),
             ". 902fbdd2b1df0c4f70b4a5d23525e932+3 0:1:A 1:1:B 2:1:C\n")
 
-    def test_checkpoint_after_put(self):
-        cwriter = TestResumableWriter()
-        with self.make_test_file(
-              't' * (cwriter.KEEP_BLOCK_SIZE + 10)) as testfile:
-            testpath = os.path.realpath(testfile.name)
-            cwriter.write_file(testpath, 'test')
-        for state in cwriter.saved_states:
-            if state.get('_current_file') == (testpath,
-                                              cwriter.KEEP_BLOCK_SIZE):
-                break
-        else:
-            self.fail("can't find state immediately after PUT to Keep")
-        self.assertIn('d45107e93f9052fa88a82fc08bb1d316+1024',  # 't' * 1024
-                      state['_current_stream_locators'])
-
     def test_basic_resume(self):
         cwriter = TestResumableWriter()
         with self.make_test_file() as testfile:
             cwriter.write_file(testfile.name, 'test')
-            last_state = cwriter.last_state()
-            resumed = TestResumableWriter.from_state(last_state)
+            resumed = TestResumableWriter.from_state(cwriter.current_state())
         self.assertEquals(cwriter.manifest_text(), resumed.manifest_text(),
                           "resumed CollectionWriter had different manifest")
 
@@ -555,7 +531,7 @@ class ArvadosCollectionsTest(ArvadosKeepLocalStoreTestCase):
             cwriter.write_file(testfile.name, 'test')
         self.assertRaises(arvados.errors.StaleWriterStateError,
                           TestResumableWriter.from_state,
-                          cwriter.last_state())
+                          cwriter.current_state())
 
     def test_resume_fails_when_dependency_mtime_changed(self):
         cwriter = TestResumableWriter()
@@ -564,14 +540,14 @@ class ArvadosCollectionsTest(ArvadosKeepLocalStoreTestCase):
             os.utime(testfile.name, (0, 0))
             self.assertRaises(arvados.errors.StaleWriterStateError,
                               TestResumableWriter.from_state,
-                              cwriter.last_state())
+                              cwriter.current_state())
 
     def test_resume_fails_when_dependency_is_nonfile(self):
         cwriter = TestResumableWriter()
         cwriter.write_file('/dev/null', 'empty')
         self.assertRaises(arvados.errors.StaleWriterStateError,
                           TestResumableWriter.from_state,
-                          cwriter.last_state())
+                          cwriter.current_state())
 
     def test_resume_fails_when_dependency_size_changed(self):
         cwriter = TestResumableWriter()
@@ -583,41 +559,16 @@ class ArvadosCollectionsTest(ArvadosKeepLocalStoreTestCase):
             os.utime(testfile.name, (orig_mtime, orig_mtime))
             self.assertRaises(arvados.errors.StaleWriterStateError,
                               TestResumableWriter.from_state,
-                              cwriter.last_state())
+                              cwriter.current_state())
 
     def test_resume_fails_with_expired_locator(self):
         cwriter = TestResumableWriter()
-        with self.make_test_file() as testfile:
-            cwriter.write_file(testfile.name, 'test')
-            cwriter.finish_current_stream()
-            state = cwriter.last_state()
-            # Get the last locator, remove any permission hint, and add
-            # an expired one.
-            new_loc = state['_current_stream_locators'][-1].split('+A', 1)[0]
-            state['_current_stream_locators'][-1] = "{}+A{}@10000000".format(
-                new_loc, 'a' * 40)
-            self.assertRaises(arvados.errors.StaleWriterStateError,
-                              TestResumableWriter.from_state, state)
-
-    def test_successful_resumes(self):
-        # FIXME: This is more of an integration test than a unit test.
-        cwriter = TestResumableWriter()
-        source_tree = self.build_directory_tree(['basefile', 'subdir/subfile'])
-        with open(os.path.join(source_tree, 'long'), 'w') as longfile:
-            longfile.write('t' * (cwriter.KEEP_BLOCK_SIZE + 10))
-        cwriter.write_directory_tree(source_tree)
-        # A state for each file, plus a fourth for mid-longfile.
-        self.assertGreater(len(cwriter.saved_states), 3,
-                           "CollectionWriter didn't save enough states to test")
-
-        for state in cwriter.saved_states:
-            new_writer = TestResumableWriter.from_state(state)
-            manifests = [writer.manifest_text()
-                         for writer in (cwriter, new_writer)]
-            self.assertEquals(
-                manifests[0], manifests[1],
-                "\n".join(["manifest mismatch after resuming from state:",
-                           pprint.pformat(state), ""] + manifests))
+        state = cwriter.current_state()
+        # Add an expired locator to the state.
+        state['_current_stream_locators'].append(''.join([
+                    'a' * 32, '+A', 'b' * 40, '@', '10000000']))
+        self.assertRaises(arvados.errors.StaleWriterStateError,
+                          TestResumableWriter.from_state, state)
 
     def test_arbitrary_objects_not_resumable(self):
         cwriter = TestResumableWriter()