Merge lp:~thomir-deactivatedaccount/adt-cloud-worker/trunk-fix-tarball-root-dir into lp:adt-cloud-worker

[Thomi Richards]

Make the results tarball root folder name more sensible.

Make the results tarball root folder name more sensible.

[Celso Providelo (cprov)]

Thanks Thomi,

Just a minor comments and you can land.

review: Approve

[lp:~thomir-deactivatedaccount/adt-cloud-worker/trunk-fix-tarball-root-dir]

28. By Thomi Richards on 2015-03-19

Fix comment.

=== modified file 'adt_cloud_worker/__init__.py'
--- adt_cloud_worker/__init__.py	2015-03-18 23:54:46 +0000
+++ adt_cloud_worker/__init__.py	2015-03-19 02:37:23 +0000
@@ -99,10 +99,12 @@
                 arguments = _make_adt_argument_list(adt_kwargs)
 
                 body['exit_code'] = run_adt(arguments)
-                tarball_path = _create_tarball_from_directory(result_dir)
-                # Upload the results tarball to swift with a temporary name
-                # 'adt-<req_id>/results.tgz.tmp', result checker will rename
-                # if after verification.
+                tarball_path = _create_tarball_from_directory(
+                    result_dir,
+                    'results_{}'.format(body['request_id'])
+                )
+                # Upload the results tarball to swift. This will be made public
+                # by the adt-result-checker.
                 container_name = 'adt-{}'.format(body['request_id'])
                 upload_tarball_to_swift_container(tarball_path, container_name)
         except Exception as e:
@@ -205,7 +207,7 @@
         os.environ[k.upper()] = str(v)
 
 
-def _create_tarball_from_directory(directory_to_tar):
+def _create_tarball_from_directory(directory_to_tar, tarball_root):
     """Create a gzip'd tarball of the given directory.
 
     :returns: the path to the newly created tarball. The caller is responsible
@@ -214,12 +216,11 @@
     """
     # Compress the results directory in a temporary file.
     _unused, tarball_path = tempfile.mkstemp()
-    dirname_for_tarball = os.path.basename(directory_to_tar)
     with tarfile.open(tarball_path, "w:gz") as tar:
         for entry in os.listdir(directory_to_tar):
             tar.add(
                 os.path.join(directory_to_tar, entry),
-                arcname=os.path.join(dirname_for_tarball, entry),
+                arcname=os.path.join(tarball_root, entry),
             )
     return tarball_path
 

=== modified file 'adt_cloud_worker/tests/test_cloud_worker.py'
--- adt_cloud_worker/tests/test_cloud_worker.py	2015-03-12 15:41:50 +0000
+++ adt_cloud_worker/tests/test_cloud_worker.py	2015-03-19 02:37:23 +0000
@@ -142,14 +142,14 @@
 
 class TarballCreationTests(testtools.TestCase):
 
-    def create_tarball_with_cleanup(self, directory_to_tar):
-        tarball_path = _create_tarball_from_directory(directory_to_tar)
+    def create_tarball_with_cleanup(self, directory_to_tar, tarball_root):
+        tarball_path = _create_tarball_from_directory(directory_to_tar, tarball_root)
         self.addCleanup(os.remove, tarball_path)
         return tarball_path
 
     def test_will_tar_empty_directory(self):
         with tempfile.TemporaryDirectory() as tempdir:
-            tarfile_path = self.create_tarball_with_cleanup(tempdir)
+            tarfile_path = self.create_tarball_with_cleanup(tempdir, 'root')
 
             self.assertThat(tarfile_path, FileExists())
             self.assertTrue(tarfile.is_tarfile(tarfile_path))
@@ -158,9 +158,9 @@
         with tempfile.TemporaryDirectory() as tempdir:
             with open(os.path.join(tempdir, 'testfile'), 'w') as f:
                 f.write("Some Data")
-            tarfile_path = self.create_tarball_with_cleanup(tempdir)
+            tarfile_path = self.create_tarball_with_cleanup(tempdir, 'root')
 
-            expected = os.path.join(os.path.basename(tempdir), 'testfile')
+            expected = 'root/testfile'
             self.assertThat(
                 tarfile_path,
                 TarballContains([expected])