launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/lp-codeimport:remove-local-bzr-svn-cache into lp-codeimport:master.

Commit message:
Remove local bzr-svn cache after saving it

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lp-codeimport/+git/lp-codeimport/+merge/404690

This follows up on an XXX comment from https://code.launchpad.net/~cjwatson/launchpad/bzr-svn-worker-store-cache/+merge/362757.  Removing the local cache means that code import workers don't end up accumulating local copies of the caches for all bzr-svn imports over time, which is a non-trivial amount of disk space.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lp-codeimport:remove-local-bzr-svn-cache into lp-codeimport:master.

diff --git a/lib/lp/codehosting/codeimport/tests/test_worker.py b/lib/lp/codehosting/codeimport/tests/test_worker.py
index 50fa83c..a26ea5e 100644
--- a/lib/lp/codehosting/codeimport/tests/test_worker.py
+++ b/lib/lp/codehosting/codeimport/tests/test_worker.py
@@ -1269,6 +1269,8 @@ class TestBzrSvnImport(WorkerTest, SubversionImportHelpers,
         worker.import_data_store.fetch('svn-cache.tar.gz')
         extract_tarball('svn-cache.tar.gz', '.')
         self.assertContentEqual(cache_dir_contents, os.listdir(uuid))
+        # The local cache is removed after saving it in the store.
+        self.assertFalse(os.path.exists(cache_dir))
 
     def test_getBazaarBranch_fetches_bzr_svn_cache(self):
         # BzrSvnImportWorker.getBazaarBranch fetches the tarball of the
diff --git a/lib/lp/codehosting/codeimport/worker.py b/lib/lp/codehosting/codeimport/worker.py
index 3332c03..5f4ee29 100644
--- a/lib/lp/codehosting/codeimport/worker.py
+++ b/lib/lp/codehosting/codeimport/worker.py
@@ -950,9 +950,7 @@ class BzrSvnImportWorker(PullingImportWorker):
                 os.path.dirname(cache_dir), local_name,
                 filenames=[os.path.basename(cache_dir)])
             self.import_data_store.put(local_name)
-            # XXX cjwatson 2019-02-06: Once this is behaving well on
-            # production, consider removing the local cache after pushing a
-            # copy of it to the import data store.
+            shutil.rmtree(cache_dir)
         return non_trivial