addressing @ElDeveloper comments

Author: antgonza

qiita_db/analysis.py

@@ -18,8 +18,8 @@
 # -----------------------------------------------------------------------------
 from __future__ import division
 from itertools import product
-from os.path import join
-from subprocess import Popen, PIPE
+from os.path import join, basename
+from tarfile import open as taropen
 
 from future.utils import viewitems
 from biom import load_table
@@ -798,17 +798,20 @@ def generate_tgz(self):
 
             _, analysis_mp = qdb.util.get_mountpoint('analysis')[0]
             tgz = join(analysis_mp, '%d_files.tgz' % self.id)
-            cmd = 'tar zcf %s %s' % (tgz, ' '.join(full_fps))
-
-            proc = Popen(cmd, universal_newlines=True, shell=True, stdout=PIPE,
-                         stderr=PIPE)
-            stdout, stderr = proc.communicate()
-            return_value = proc.returncode
+            try:
+                with taropen(tgz, "w:gz") as tar:
+                    for f in full_fps:
+                        tar.add(f, arcname=basename(f))
+                error_txt = ''
+                return_value = 0
+            except Exception as e:
+                error_txt = str(e)
+                return_value = 1
 
             if return_value == 0:
                 self._add_file(tgz, 'tgz')
 
-        return stdout, stderr, return_value
+        return '', error_txt, return_value
 
     def build_files(self,
                     rarefaction_depth=None,

qiita_db/support_files/patches/python_patches/41.py

@@ -1,6 +1,5 @@
-
-from subprocess import Popen, PIPE
-from os.path import exists, join
+from os.path import exists, join, basename
+from tarfile import open as taropen
 
 from qiita_db.sql_connection import TRN
 from qiita_db.artifact import Artifact
@@ -34,15 +33,8 @@
 
         tgz = to_tgz + '.tgz'
         if not exists(tgz):
-            cmd = 'tar zcf %s %s' % (tgz, to_tgz)
-            proc = Popen(cmd, universal_newlines=True, shell=True, stdout=PIPE,
-                         stderr=PIPE)
-            stdout, stderr = proc.communicate()
-            return_value = proc.returncode
-            if return_value != 0:
-                raise ValueError(
-                    "There was an error:\nstdout:\n%s\n\nstderr:\n%s"
-                    % (stdout, stderr))
+            with taropen(tgz, "w:gz") as tar:
+                tar.add(to_tgz, arcname=basename(to_tgz))
 
         a_id = a.id
         # Add the new tgz file to the artifact.
@@ -88,15 +80,9 @@
         if not exists(tgz):
             full_fps = [join(get_mountpoint_path_by_id(mid), f)
                         for f, mid in fps]
-            cmd = 'tar zcf %s %s' % (tgz, ' '.join(full_fps))
-            proc = Popen(cmd, universal_newlines=True, shell=True, stdout=PIPE,
-                         stderr=PIPE)
-            stdout, stderr = proc.communicate()
-            return_value = proc.returncode
-            if return_value != 0:
-                raise ValueError(
-                    "There was an error:\nstdout:\n%s\n\nstderr:\n%s"
-                    % (stdout, stderr))
+            with taropen(tgz, "w:gz") as tar:
+                for f in full_fps:
+                    tar.add(f, arcname=basename(f))
 
         # Add the new tgz file to the analysis.
         fp_ids = insert_filepaths([(tgz, tgz_id)], analysis_id, 'analysis',

qiita_plugins/target_gene/tgp/pick_otus.py

@@ -6,9 +6,10 @@
 # The full license is in the file LICENSE, distributed with this software.
 # -----------------------------------------------------------------------------
 
-from os.path import join
+from os.path import join, basename
 from functools import partial
 from glob import glob
+from tarfile import open as taropen
 
 from qiita_client import format_payload
 
@@ -103,9 +104,8 @@ def generate_sortmerna_tgz(out_dir):
     """
     to_tgz = join(out_dir, 'sortmerna_picked_otus')
     tgz = to_tgz + '.tgz'
-    cmd = 'tar zcf %s %s' % (tgz, to_tgz)
-
-    return cmd
+    with taropen(tgz, "w:gz") as tar:
+        tar.add(to_tgz, arcname=basename(to_tgz))
 
 
 def generate_artifact_info(pick_out):
@@ -189,11 +189,10 @@ def pick_closed_reference_otus(qclient, job_id, parameters, out_dir):
 
     qclient.update_job_step(job_id,
                             "Step 4 of 4: Generating tgz sortmerna folder")
-    command = generate_sortmerna_tgz(pick_out)
-    std_out, std_err, return_value = system_call(command)
-    if return_value != 0:
-        error_msg = ("Error while tgz failures:\nStd out: %s\nStd err: %s"
-                     % (std_out, std_err))
+    try:
+        generate_sortmerna_tgz(pick_out)
+    except Exception as e:
+        error_msg = ("Error while tgz failures:\nError: %s" % str(e))
         return format_payload(False, error_msg=error_msg)
 
     artifacts_info = generate_artifact_info(pick_out)

qiita_plugins/target_gene/tgp/tests/test_pick_otus.py

@@ -8,7 +8,7 @@
 
 from unittest import TestCase, main
 from os.path import isdir, exists, join
-from os import remove, close
+from os import remove, close, mkdir
 from shutil import rmtree
 from tempfile import mkstemp, mkdtemp
 
@@ -70,10 +70,8 @@ def test_generate_pick_closed_reference_otus_cmd(self):
     def test_generate_sortmerna_tgz(self):
         outdir = mkdtemp()
         self._clean_up_files.append(outdir)
-        obs = generate_sortmerna_tgz(outdir)
-        exp = ("tar zcf %s/sortmerna_picked_otus.tgz "
-               "%s/sortmerna_picked_otus" % (outdir, outdir))
-        self.assertEqual(obs, exp)
+        mkdir(join(outdir, 'sortmerna_picked_otus'))
+        self.assertIsNone(generate_sortmerna_tgz(outdir))
 
     def test_generate_pick_closed_reference_otus_cmd_valueerror(self):
         filepaths = [('/directory/seqs.log', 'log'),