Fixing minor issues (#3020)

* fixing minor issues

* fix tests

* some minor JS fixes

* rm TRN in def release_validators(job)

* fix self.step for validators

* order LIBRARY_DESCRIPTOR and upper LIBRARY_STRATEGY

Author: antgonza

qiita_db/processing_job.py

@@ -1007,75 +1007,74 @@ def release(self):
 
     def release_validators(self):
         """Allows all the validator job spawned by this job to complete"""
-        with qdb.sql_connection.TRN:
-            if self.command.software.type not in ('artifact transformation',
-                                                  'private'):
-                raise qdb.exceptions.QiitaDBOperationNotPermittedError(
-                    "Only artifact transformation and private jobs can "
-                    "release validators")
-
-            # Check if all the validators are completed. Validator jobs can be
-            # in two states when completed: 'waiting' in case of success
-            # or 'error' otherwise
-
+        if self.command.software.type not in ('artifact transformation',
+                                              'private'):
+            raise qdb.exceptions.QiitaDBOperationNotPermittedError(
+                "Only artifact transformation and private jobs can "
+                "release validators")
+
+        # Check if all the validators are completed. Validator jobs can be
+        # in two states when completed: 'waiting' in case of success
+        # or 'error' otherwise
+
+        validator_ids = ['%s [%s]' % (j.id, j.external_id)
+                         for j in self.validator_jobs
+                         if j.status not in ['waiting', 'error']]
+
+        # Active polling - wait until all validator jobs are completed
+        # TODO: As soon as we see one errored validator, we should kill
+        # the other jobs and exit early. Don't wait for all of the jobs
+        # to complete.
+        while validator_ids:
+            jids = ', '.join(validator_ids)
+            self.step = ("Validating outputs (%d remaining) via "
+                         "job(s) %s" % (len(validator_ids), jids))
+            sleep(10)
             validator_ids = ['%s [%s]' % (j.id, j.external_id)
                              for j in self.validator_jobs
                              if j.status not in ['waiting', 'error']]
 
-            # Active polling - wait until all validator jobs are completed
-            # TODO: As soon as we see one errored validator, we should kill
-            # the other jobs and exit early. Don't wait for all of the jobs
-            # to complete.
-            while validator_ids:
-                jids = ', '.join([j[0] for j in validator_ids])
-                self.step = ("Validating outputs (%d remaining) via "
-                             "job(s) %s" % (len(validator_ids), jids))
-                sleep(10)
-                validator_ids = ['%s [%s]' % (j.id, j.external_id)
-                                 for j in self.validator_jobs
-                                 if j.status not in ['waiting', 'error']]
-
-            # Check if any of the validators errored
-            errored = [j for j in self.validator_jobs
-                       if j.status == 'error']
-            if errored:
-                # At least one of the validators failed, Set the rest of the
-                # validators and the current job as failed
-                waiting = [j.id for j in self.validator_jobs
-                           if j.status == 'waiting']
-
-                common_error = "\n".join(
-                    ["Validator %s error message: %s" % (j.id, j.log.msg)
-                     for j in errored])
-
-                val_error = "%d sister validator jobs failed: %s" % (
-                    len(errored), common_error)
-                for j in waiting:
-                    ProcessingJob(j)._set_error(val_error)
-
-                self._set_error('%d validator jobs failed: %s'
-                                % (len(errored), common_error))
-            else:
-                mapping = {}
-                # Loop through all validator jobs and release them, allowing
-                # to create the artifacts. Note that if any artifact creation
-                # fails, the rollback operation will make sure that the
-                # previously created artifacts are not in there
-                for vjob in self.validator_jobs:
-                    mapping.update(vjob.release())
-
-                if mapping:
-                    sql = """INSERT INTO
-                                qiita.artifact_output_processing_job
-                                (artifact_id, processing_job_id,
-                                command_output_id)
-                             VALUES (%s, %s, %s)"""
-                    sql_args = [[aid, self.id, outid]
-                                for outid, aid in mapping.items()]
+        # Check if any of the validators errored
+        errored = [j for j in self.validator_jobs
+                   if j.status == 'error']
+        if errored:
+            # At least one of the validators failed, Set the rest of the
+            # validators and the current job as failed
+            waiting = [j.id for j in self.validator_jobs
+                       if j.status == 'waiting']
+
+            common_error = "\n".join(
+                ["Validator %s error message: %s" % (j.id, j.log.msg)
+                 for j in errored])
+
+            val_error = "%d sister validator jobs failed: %s" % (
+                len(errored), common_error)
+            for j in waiting:
+                ProcessingJob(j)._set_error(val_error)
+
+            self._set_error('%d validator jobs failed: %s'
+                            % (len(errored), common_error))
+        else:
+            mapping = {}
+            # Loop through all validator jobs and release them, allowing
+            # to create the artifacts. Note that if any artifact creation
+            # fails, the rollback operation will make sure that the
+            # previously created artifacts are not in there
+            for vjob in self.validator_jobs:
+                mapping.update(vjob.release())
+
+            if mapping:
+                sql = """INSERT INTO
+                            qiita.artifact_output_processing_job
+                            (artifact_id, processing_job_id,
+                            command_output_id)
+                         VALUES (%s, %s, %s)"""
+                sql_args = [[aid, self.id, outid]
+                            for outid, aid in mapping.items()]
+                with qdb.sql_connection.TRN:
                     qdb.sql_connection.TRN.add(sql, sql_args, many=True)
-
-                    self._update_and_launch_children(mapping)
-                self._set_status('success')
+                self._update_and_launch_children(mapping)
+            self._set_status('success')
 
     def _complete_artifact_definition(self, artifact_data):
         """"Performs the needed steps to complete an artifact definition job

qiita_pet/handlers/api_proxy/processing.py

@@ -240,7 +240,8 @@ def job_ajax_get_req(job_id):
     job_error = job.log.msg if job.log is not None else None
     return {'status': 'success',
             'message': '',
-            'job_id': '%s [%s]' % (job.id, job.external_id),
+            'job_id': job.id,
+            'job_external_id': job.external_id,
             'job_status': job_status,
             'job_step': job.step,
             'job_parameters': job.parameters.values,

qiita_pet/handlers/api_proxy/tests/test_processing.py

@@ -82,7 +82,8 @@ def test_job_ajax_get_req(self):
         exp = {
             'status': 'success',
             'message': '',
-            'job_id': "063e553b-327c-4818-ab4a-adfe58e49860 [Not Available]",
+            'job_id': "063e553b-327c-4818-ab4a-adfe58e49860",
+            'job_external_id': "Not Available",
             'job_status': "queued",
             'job_step': None,
             'job_error': None,

qiita_pet/handlers/study_handlers/tests/test_processing.py

@@ -32,7 +32,8 @@ def test_get(self):
         exp = {
             'status': 'success',
             'message': '',
-            'job_id': "063e553b-327c-4818-ab4a-adfe58e49860 [Not Available]",
+            'job_id': "063e553b-327c-4818-ab4a-adfe58e49860",
+            'job_external_id': 'Not Available',
             'job_status': "queued",
             'job_step': None,
             'job_error': None,

qiita_pet/static/js/networkVue.js

@@ -349,10 +349,11 @@ Vue.component('processing-graph', {
         vm.updateGraph();
       })
         .fail(function(object, status, error_msg) {
-          $('#run-btn').attr('disabled', false);
-          $('#run-btn').innerHTML = '<span class="glyphicon glyphicon-play"></span> Run';
           bootstrapAlert("Error submitting workflow: " + object.statusText, "danger");
         });
+      // return button to regular state
+      $('#run-btn').attr('disabled', false);
+      $('#run-btn').html('<span class="glyphicon glyphicon-play"></span> Run');
     },
 
     /**
@@ -371,7 +372,7 @@ Vue.component('processing-graph', {
         $("#processing-results").empty();
 
         // Create the header of the page
-        var h = $("<h3>").text('Job ' + data['job_id'] + ' ').appendTo("#processing-results");
+        var h = $("<h3>").text('Job ' + data['job_id'] + ' [' + data['job_external_id'] + '] ').appendTo("#processing-results");
 
         // Only add the delete job button if the job is "in_construction"
         // or "error"

qiita_pet/templates/software.html

@@ -19,11 +19,11 @@ <h5>
       <a href="https://qiita.ucsd.edu/static/doc/html/processingdata/processing-recommendations.html">Here</a> you will find our current processing recommendations.
     </h5>
 
-    <table class="table-bordered" width="50%">
+    <table class="table-bordered" width="90%">
       <thead>
         <tr>
-          <th>Plugin</th>
-          <th>Commands</th>
+          <th width="20%">Plugin</th>
+          <th width="80%">Commands</th>
         </tr>
       </thead>
       <tbody>
@@ -45,8 +45,8 @@ <h5>
             <table class="display table-bordered table-hover" width="100%">
               <thead>
                 <tr>
-                  <th>Command Name</th>
-                  <th>Resource Allocation</th>
+                  <th width="55%">Command Name</th>
+                  <th width="44%">Resource Allocation</th>
                 </tr>
               </thead>
               <tbody>

qiita_ware/ebi.py

@@ -526,6 +526,9 @@ def generate_experiment_xml(self, samples=None):
             library_name = ET.SubElement(library_descriptor, 'LIBRARY_NAME')
             library_name.text = self._get_library_name(sample_name)
 
+            lg = ET.SubElement(library_descriptor, 'LIBRARY_STRATEGY')
+            lg.text = escape(clean_whitespace(library_strategy.upper()))
+
             # hardcoding some values,
             # see https://github.com/biocore/qiita/issues/1485
             library_source = ET.SubElement(library_descriptor,
@@ -546,9 +549,6 @@ def generate_experiment_xml(self, samples=None):
             lcp.text = escape(clean_whitespace(
                 sample_prep.pop('library_construction_protocol')))
 
-            lg = ET.SubElement(library_descriptor, 'LIBRARY_STRATEGY')
-            lg.text = library_strategy
-
             self._generate_spot_descriptor(design, platform)
 
             platform_element = ET.SubElement(experiment, 'PLATFORM')

qiita_ware/private_plugin.py

@@ -75,10 +75,9 @@ def release_validators(job):
     job : qiita_db.processing_job.ProcessingJob
         The processing job with the information of the parent job
     """
-    with qdb.sql_connection.TRN:
-        qdb.processing_job.ProcessingJob(
-            job.parameters.values['job']).release_validators()
-        job._set_status('success')
+    qdb.processing_job.ProcessingJob(
+        job.parameters.values['job']).release_validators()
+    job._set_status('success')
 
 
 def submit_to_VAMPS(job):

qiita_ware/test/test_ebi.py

@@ -532,7 +532,7 @@ def test_generate_experiment_xml(self):
 
         # changing investigation_type to test user defined terms, first let's
         # create a new term
-        new_term = 'Ultimate Term'
+        new_term = 'ULTIMATE TERM'
         ena_ontology = Ontology(convert_to_id('ENA', 'ontology'))
         ena_ontology.add_user_defined_term(new_term)
         # set the preparation with the new term
@@ -1359,12 +1359,12 @@ def test_parse_EBI_reply(self):
 %(study_id)s.Sample1" />
       <LIBRARY_DESCRIPTOR>
         <LIBRARY_NAME>%(study_id)s.Sample1</LIBRARY_NAME>
+        <LIBRARY_STRATEGY>METAGENOMICS</LIBRARY_STRATEGY>
         <LIBRARY_SOURCE>METAGENOMIC</LIBRARY_SOURCE>
         <LIBRARY_SELECTION>PCR</LIBRARY_SELECTION>
         <LIBRARY_LAYOUT><SINGLE /></LIBRARY_LAYOUT>
         <LIBRARY_CONSTRUCTION_PROTOCOL>Protocol ABC
         </LIBRARY_CONSTRUCTION_PROTOCOL>
-        <LIBRARY_STRATEGY>Metagenomics</LIBRARY_STRATEGY>
       </LIBRARY_DESCRIPTOR>
     </DESIGN>
     <PLATFORM>
@@ -1394,12 +1394,12 @@ def test_parse_EBI_reply(self):
 %(study_id)s.Sample2" />
       <LIBRARY_DESCRIPTOR>
         <LIBRARY_NAME>%(study_id)s.Sample2</LIBRARY_NAME>
+        <LIBRARY_STRATEGY>METAGENOMICS</LIBRARY_STRATEGY>
         <LIBRARY_SOURCE>METAGENOMIC</LIBRARY_SOURCE>
         <LIBRARY_SELECTION>PCR</LIBRARY_SELECTION>
         <LIBRARY_LAYOUT><SINGLE /></LIBRARY_LAYOUT>
         <LIBRARY_CONSTRUCTION_PROTOCOL>Protocol ABC
         </LIBRARY_CONSTRUCTION_PROTOCOL>
-        <LIBRARY_STRATEGY>Metagenomics</LIBRARY_STRATEGY>
       </LIBRARY_DESCRIPTOR>
     </DESIGN>
     <PLATFORM>
@@ -1429,12 +1429,12 @@ def test_parse_EBI_reply(self):
 %(study_id)s.Sample3" />
       <LIBRARY_DESCRIPTOR>
         <LIBRARY_NAME>%(study_id)s.Sample3</LIBRARY_NAME>
+        <LIBRARY_STRATEGY>METAGENOMICS</LIBRARY_STRATEGY>
         <LIBRARY_SOURCE>METAGENOMIC</LIBRARY_SOURCE>
         <LIBRARY_SELECTION>PCR</LIBRARY_SELECTION>
         <LIBRARY_LAYOUT><SINGLE /></LIBRARY_LAYOUT>
         <LIBRARY_CONSTRUCTION_PROTOCOL>Protocol ABC
         </LIBRARY_CONSTRUCTION_PROTOCOL>
-        <LIBRARY_STRATEGY>Metagenomics</LIBRARY_STRATEGY>
       </LIBRARY_DESCRIPTOR>
     </DESIGN>
     <PLATFORM>
@@ -1470,6 +1470,7 @@ def test_parse_EBI_reply(self):
       <SAMPLE_DESCRIPTOR accession="ERS000008" />
       <LIBRARY_DESCRIPTOR>
         <LIBRARY_NAME>1.SKB2.640194</LIBRARY_NAME>
+        <LIBRARY_STRATEGY>METAGENOMICS</LIBRARY_STRATEGY>
         <LIBRARY_SOURCE>METAGENOMIC</LIBRARY_SOURCE>
         <LIBRARY_SELECTION>PCR</LIBRARY_SELECTION>
         <LIBRARY_LAYOUT><SINGLE /></LIBRARY_LAYOUT>
@@ -1484,7 +1485,6 @@ def test_parse_EBI_reply(self):
 multiplexing of up to 1,500 samples per lane, and both PCR primers contain \
 sequencer adapter regions.
         </LIBRARY_CONSTRUCTION_PROTOCOL>
-        <LIBRARY_STRATEGY>Metagenomics</LIBRARY_STRATEGY>
       </LIBRARY_DESCRIPTOR>
     </DESIGN>
     <PLATFORM>
@@ -1559,6 +1559,7 @@ def test_parse_EBI_reply(self):
       <SAMPLE_DESCRIPTOR accession="ERS000024" />
       <LIBRARY_DESCRIPTOR>
         <LIBRARY_NAME>1.SKB3.640195</LIBRARY_NAME>
+        <LIBRARY_STRATEGY>METAGENOMICS</LIBRARY_STRATEGY>
         <LIBRARY_SOURCE>METAGENOMIC</LIBRARY_SOURCE>
         <LIBRARY_SELECTION>PCR</LIBRARY_SELECTION>
         <LIBRARY_LAYOUT><SINGLE /></LIBRARY_LAYOUT>
@@ -1573,7 +1574,6 @@ def test_parse_EBI_reply(self):
 multiplexing of up to 1,500 samples per lane, and both PCR primers contain \
 sequencer adapter regions.
         </LIBRARY_CONSTRUCTION_PROTOCOL>
-        <LIBRARY_STRATEGY>Metagenomics</LIBRARY_STRATEGY>
       </LIBRARY_DESCRIPTOR>
     </DESIGN>
     <PLATFORM>