some improvements (#3445)

* some changes

* rm circular import

* AMPLICON

* AMPLICON tests

* reservation

* fix errors

* fix qiita_db tests

* fix trace

* addressing @charles-cowart comments

Author: antgonza

qiita_db/analysis.py

@@ -215,6 +215,22 @@ def create(cls, owner, name, description, from_default=False,
         job.submit()
         return instance
 
+    @classmethod
+    def delete_analysis_artifacts(cls, _id):
+        """Deletes the artifacts linked to an artifact and then the analysis
+
+        Parameters
+        ----------
+        _id : int
+            The analysis id
+        """
+        analysis = cls(_id)
+        aids = [a.id for a in analysis.artifacts if not a.parents]
+        aids.sort(reverse=True)
+        for aid in aids:
+            qdb.artifact.Artifact.delete(aid)
+        cls.delete(analysis.id)
+
     @classmethod
     def delete(cls, _id):
         """Deletes an analysis

qiita_db/handlers/tests/test_processing_job.py

@@ -233,9 +233,9 @@ def test_post_job_success(self):
         self.assertIsNotNone(cj)
         # additionally we can test that job.print_trace is correct
         self.assertEqual(job.trace, [
-            f'{job.id} [Not Available]: Validate | '
+            f'{job.id} [Not Available] (success): Validate | '
             '-p qiita -N 1 -n 1 --mem 90gb --time 150:00:00 --nice=10000',
-            f'  {cj.id} [{cj.external_id}] | '
+            f'  {cj.id} [{cj.external_id}] (success)| '
             '-p qiita -N 1 -n 1 --mem 16gb --time 10:00:00 --nice=10000'])
 
     def test_post_job_success_with_archive(self):

qiita_db/metadata_template/prep_template.py

@@ -135,7 +135,7 @@ def create(cls, md_template, study, data_type, investigation_type=None,
             # data_type being created - if possible
             if investigation_type is None:
                 if data_type_str in TARGET_GENE_DATA_TYPES:
-                    investigation_type = 'Amplicon'
+                    investigation_type = 'AMPLICON'
                 elif data_type_str == 'Metagenomic':
                     investigation_type = 'WGS'
                 elif data_type_str == 'Metatranscriptomic':
@@ -280,8 +280,22 @@ def delete(cls, id_):
             qdb.sql_connection.TRN.add(sql, args)
             archived_artifacts = set(
                 qdb.sql_connection.TRN.execute_fetchflatten())
+            ANALYSIS = qdb.analysis.Analysis
             if archived_artifacts:
                 for aid in archived_artifacts:
+                    # before we can delete the archived artifact, we need
+                    # to delete the analyses where they were used.
+                    sql = """SELECT analysis_id
+                             FROM qiita.analysis
+                             WHERE analysis_id IN (
+                                SELECT DISTINCT analysis_id
+                                FROM qiita.analysis_sample
+                                WHERE artifact_id IN %s)"""
+                    qdb.sql_connection.TRN.add(sql, [tuple([aid])])
+                    analyses = set(
+                        qdb.sql_connection.TRN.execute_fetchflatten())
+                    for _id in analyses:
+                        ANALYSIS.delete_analysis_artifacts(_id)
                     qdb.artifact.Artifact.delete(aid)
 
             # Delete the prep template filepaths

qiita_db/metadata_template/test/test_prep_template.py

@@ -911,7 +911,7 @@ def _common_creation_checks(self, pt, fp_count, name):
         self.assertEqual(pt.data_type(), self.data_type)
         self.assertEqual(pt.data_type(ret_id=True), self.data_type_id)
         self.assertEqual(pt.artifact, None)
-        self.assertEqual(pt.investigation_type, 'Amplicon')
+        self.assertEqual(pt.investigation_type, 'AMPLICON')
         self.assertEqual(pt.study_id, self.test_study.id)
         self.assertEqual(pt.status, "sandbox")
         exp_sample_ids = {'%s.SKB8.640193' % self.test_study.id,
@@ -1076,7 +1076,7 @@ def test_create_warning(self):
         self.assertEqual(pt.data_type(), self.data_type)
         self.assertEqual(pt.data_type(ret_id=True), self.data_type_id)
         self.assertEqual(pt.artifact, None)
-        self.assertEqual(pt.investigation_type, 'Amplicon')
+        self.assertEqual(pt.investigation_type, 'AMPLICON')
         self.assertEqual(pt.study_id, self.test_study.id)
         self.assertEqual(pt.status, 'sandbox')
         exp_sample_ids = {'%s.SKB8.640193' % self.test_study.id,
@@ -1247,7 +1247,7 @@ def test_investigation_type_setter(self):
         """Able to update the investigation type"""
         pt = qdb.metadata_template.prep_template.PrepTemplate.create(
             self.metadata, self.test_study, self.data_type_id)
-        self.assertEqual(pt.investigation_type, 'Amplicon')
+        self.assertEqual(pt.investigation_type, 'AMPLICON')
         pt.investigation_type = "Other"
         self.assertEqual(pt.investigation_type, 'Other')
         with self.assertRaises(qdb.exceptions.QiitaDBColumnError):

qiita_db/processing_job.py

@@ -2053,23 +2053,25 @@ def complete_processing_job(self):
     def trace(self):
         """ Returns as a text array the full trace of the job, from itself
             to validators and complete jobs"""
-        lines = [f'{self.id} [{self.external_id}]: '
+        lines = [f'{self.id} [{self.external_id}] ({self.status}): '
                  f'{self.command.name} | {self.resource_allocation_info}']
         cjob = self.complete_processing_job
         if cjob is not None:
-            lines.append(f'  {cjob.id} [{cjob.external_id}] | '
+            lines.append(f'  {cjob.id} [{cjob.external_id}] ({cjob.status})| '
                          f'{cjob.resource_allocation_info}')
             vjob = self.release_validator_job
             if vjob is not None:
                 lines.append(f'    {vjob.id} [{vjob.external_id}] '
-                             f'| {vjob.resource_allocation_info}')
+                             f' ({vjob.status}) | '
+                             f'{vjob.resource_allocation_info}')
         for v in self.validator_jobs:
-            lines.append(f'     {v.id} [{v.external_id}]: '
+            lines.append(f'     {v.id} [{v.external_id}] ({v.status}): '
                          f'{v.command.name} | {v.resource_allocation_info}')
             cjob = v.complete_processing_job
             if cjob is not None:
                 lines.append(f'         {cjob.id} [{cjob.external_id}] '
-                             f'| {cjob.resource_allocation_info}')
+                             f'({cjob.status}) | '
+                             f'{cjob.resource_allocation_info}')
         return lines
 
 

qiita_db/support_files/patches/93.sql

@@ -55,3 +55,10 @@ CREATE INDEX IF NOT EXISTS processing_job_command_parameters_payload ON qiita.pr
 
 -- After the changes
 -- 18710.404 ms
+
+--
+
+-- Nov 5, 2024
+-- Addding contraints for the slurm_reservation column
+ALTER TABLE qiita.analysis DROP CONSTRAINT IF EXISTS analysis_slurm_reservation_valid_chars;
+ALTER TABLE qiita.analysis ADD CONSTRAINT analysis_slurm_reservation_valid_chars CHECK ( slurm_reservation ~ '^[a-zA-Z0-9_]*$' );

qiita_db/support_files/populate_test_db.sql

@@ -1336,7 +1336,7 @@ INSERT INTO qiita.study_users VALUES (1, 'shared@foo.bar');
 
 INSERT INTO qiita.term VALUES (2052508974, 999999999, NULL, 'WGS', 'ENA:0000059', NULL, NULL, NULL, NULL, NULL, false);
 INSERT INTO qiita.term VALUES (2052508975, 999999999, NULL, 'Metagenomics', 'ENA:0000060', NULL, NULL, NULL, NULL, NULL, false);
-INSERT INTO qiita.term VALUES (2052508976, 999999999, NULL, 'Amplicon', 'ENA:0000061', NULL, NULL, NULL, NULL, NULL, false);
+INSERT INTO qiita.term VALUES (2052508976, 999999999, NULL, 'AMPLICON', 'ENA:0000061', NULL, NULL, NULL, NULL, NULL, false);
 INSERT INTO qiita.term VALUES (2052508984, 999999999, NULL, 'RNA-Seq', 'ENA:0000070', NULL, NULL, NULL, NULL, NULL, false);
 INSERT INTO qiita.term VALUES (2052508987, 999999999, NULL, 'Other', 'ENA:0000069', NULL, NULL, NULL, NULL, NULL, false);
 

qiita_db/test/test_analysis.py

@@ -688,7 +688,7 @@ def test_is_public_make_public(self):
     def test_slurm_reservation(self):
         analysis = qdb.analysis.Analysis(1)
         self.assertIsNone(analysis.slurm_reservation)
-        text = 'this is a test!'
+        text = 'thisisatest'
         analysis.slurm_reservation = text
         self.assertEqual(analysis._slurm_reservation(), [text])
         self.assertIsNone(analysis.slurm_reservation)

qiita_db/test/test_artifact.py

@@ -23,7 +23,6 @@
 from qiita_core.util import qiita_test_checker
 from qiita_core.testing import wait_for_processing_job
 import qiita_db as qdb
-from qiita_ware.private_plugin import _delete_analysis_artifacts
 
 
 class ArtifactTestsReadOnly(TestCase):
@@ -1559,9 +1558,9 @@ def test_archive(self):
         qdb.sql_connection.perform_as_transaction(sql)
         sql = "UPDATE qiita.artifact SET visibility_id = 1"
         qdb.sql_connection.perform_as_transaction(sql)
-        _delete_analysis_artifacts(qdb.analysis.Analysis(1))
-        _delete_analysis_artifacts(qdb.analysis.Analysis(2))
-        _delete_analysis_artifacts(qdb.analysis.Analysis(3))
+        qdb.analysis.Analysis.delete_analysis_artifacts(1)
+        qdb.analysis.Analysis.delete_analysis_artifacts(2)
+        qdb.analysis.Analysis.delete_analysis_artifacts(3)
         for aid in [3, 2, 1]:
             A.delete(aid)
 

qiita_db/test/test_ontology.py

@@ -35,7 +35,7 @@ def testShortNameProperty(self):
     def testTerms(self):
         obs = self.ontology.terms
         self.assertEqual(
-            obs, ['WGS', 'Metagenomics', 'Amplicon', 'RNA-Seq', 'Other'])
+            obs, ['WGS', 'Metagenomics', 'AMPLICON', 'RNA-Seq', 'Other'])
 
     def test_user_defined_terms(self):
         obs = self.ontology.user_defined_terms

qiita_db/util.py

@@ -2334,7 +2334,7 @@ def send_email(to, subject, body):
     msg = MIMEMultipart()
     msg['From'] = qiita_config.smtp_email
     msg['To'] = to
-    msg['Subject'] = subject
+    msg['Subject'] = subject.strip()
     msg.attach(MIMEText(body, 'plain'))
 
     # connect to smtp server, using ssl if needed
@@ -2496,9 +2496,9 @@ def _resource_allocation_plot_helper(
     ax.set_ylabel(curr)
     ax.set_xlabel(col_name)
 
-    # 100 - number of maximum iterations, 3 - number of failures we tolerate
+    # 50 - number of maximum iterations, 3 - number of failures we tolerate
     best_model, options = _resource_allocation_calculate(
-        df, x_data, y_data, models, curr, col_name, 100, 3)
+        df, x_data, y_data, models, curr, col_name, 50, 3)
     k, a, b = options.x
     x_plot = np.array(sorted(df[col_name].unique()))
     y_plot = best_model(x_plot, k, a, b)
@@ -2593,6 +2593,8 @@ def _resource_allocation_calculate(
             failures_df = _resource_allocation_failures(
                 df, k, a, b, model, col_name, type_)
             y_plot = model(x, k, a, b)
+            if not any(y_plot):
+                continue
             cmax = max(y_plot)
             cmin = min(y_plot)
             failures = failures_df.shape[0]
@@ -2834,13 +2836,17 @@ def merge_rows(rows):
         wait_time = (
             datetime.strptime(rows.iloc[0]['Start'], date_fmt)
             - datetime.strptime(rows.iloc[0]['Submit'], date_fmt))
-        tmp = rows.iloc[1].copy()
+        if rows.shape[0] >= 2:
+            tmp = rows.iloc[1].copy()
+        else:
+            tmp = rows.iloc[0].copy()
         tmp['WaitTime'] = wait_time
         return tmp
 
     slurm_data['external_id'] = slurm_data['JobID'].apply(
                                             lambda x: int(x.split('.')[0]))
     slurm_data['external_id'] = slurm_data['external_id'].ffill()
+
     slurm_data = slurm_data.groupby(
             'external_id').apply(merge_rows).reset_index(drop=True)
 

qiita_pet/handlers/analysis_handlers/tests/test_base_handlers.py

@@ -207,7 +207,7 @@ def test_patch(self):
         self.assertEqual(analysis._slurm_reservation(), [''])
 
         # now, let's change it to something different
-        reservation = 'my-reservation'
+        reservation = 'myreservation'
         arguments = {
             'op': 'replace', 'path': 'reservation', 'value': reservation}
         self.patch(f'/analysis/description/{analysis.id}/', data=arguments)

qiita_pet/handlers/api_proxy/tests/test_prep_template.py

@@ -38,7 +38,7 @@ class TestPrepAPIReadOnly(TestCase):
     def test_get_ENA_ontology(self):
         obs = _get_ENA_ontology()
         exp = {
-            'ENA': ['Amplicon', 'Metagenomics', 'RNA-Seq', 'WGS', 'Other'],
+            'ENA': ['AMPLICON', 'Metagenomics', 'RNA-Seq', 'WGS', 'Other'],
             'User': []}
         self.assertEqual(obs, exp)
 
@@ -53,7 +53,7 @@ def test_new_prep_template_get_req(self):
                            'Multiomic', 'Proteomic', 'Transcriptomics',
                            'Viromics'],
             'ontology': {
-                'ENA': ['Amplicon', 'Metagenomics', 'RNA-Seq', 'WGS', 'Other'],
+                'ENA': ['AMPLICON', 'Metagenomics', 'RNA-Seq', 'WGS', 'Other'],
                 'User': []}}
 
         self.assertEqual(obs, exp)
@@ -73,7 +73,7 @@ def test_prep_template_ajax_get_req(self):
                'num_columns': 22,
                'investigation_type': 'Metagenomics',
                'ontology': {
-                   'ENA': ['Amplicon', 'Metagenomics', 'RNA-Seq', 'WGS',
+                   'ENA': ['AMPLICON', 'Metagenomics', 'RNA-Seq', 'WGS',
                            'Other'],
                    'User': []},
                'artifact_attached': True,

qiita_pet/templates/resources.html

@@ -21,14 +21,14 @@ <h3>Please choose software, version, and command to view the data.</h3>
             <option value="">Select Software</option>
         </select>
       </div>
-    
+
         <div class="form-group">
             <label for="version">Version:</label>
             <select id="version" name="version" class="form-control">
                 <option value="">Select Version</option>
             </select>
         </div>
-    
+
         <div class="form-group">
             <label for="command">Command:</label>
             <select id="command" name="command" class="form-control">
@@ -89,15 +89,15 @@ <h3>Generated on: {{time}} </h3>
             </tr>
         </table>
     </div>
-    
+
 </div>
 
 <script>
 
     function toggleDataVisibility(showData) {
         const defaultMessage = document.getElementById('default-message');
         const dataContainer = document.getElementById('data-container');
-        
+
         if (showData) {
             defaultMessage.style.display = 'none';
             dataContainer.style.display = 'block';
@@ -106,17 +106,15 @@ <h3>Generated on: {{time}} </h3>
             dataContainer.style.display = 'none';
         }
     }
-    
+
     // Call this function on initial load
     {% if initial_load %}
         toggleDataVisibility(false);
     {% else %}
         toggleDataVisibility(true);
     {% end %}
 
-  const commandsConst = JSON.parse('{% raw commands %}');
-  console.log(commandsConst);
-
+  const commandsConst = JSON.parse(`{% raw commands %}`);
   const softwareSelect = document.getElementById('software');
   const versionSelect = document.getElementById('version');
   const commandSelect = document.getElementById('command');
@@ -133,7 +131,7 @@ <h3>Generated on: {{time}} </h3>
   function populateVersions(software) {
       versionSelect.innerHTML = '<option value="">Select Version</option>';
       commandSelect.innerHTML = '<option value="">Select Command</option>';
-      
+
       if (software && commandsConst[software]) {
           for (const version in commandsConst[software]) {
               const option = document.createElement('option');
@@ -147,7 +145,7 @@ <h3>Generated on: {{time}} </h3>
 
   function populateCommands(software, version) {
       commandSelect.innerHTML = '<option value="">Select Command</option>';
-      
+
       if (software && version && commandsConst[software][version]) {
           commandsConst[software][version].forEach(command => {
               const option = document.createElement('option');
@@ -167,7 +165,6 @@ <h3>Generated on: {{time}} </h3>
 
     $.post(window.location.href, JSON.stringify(data), function(response, textStatus, jqXHR) {
         if (response.status === "success") {
-            console.log(response.time)
             // Toggle visibility based on the response
             toggleDataVisibility(true);
 
@@ -227,7 +224,7 @@ <h3>Generated on: {{time}} </h3>
             }
 
             bootstrapAlert("Data updated successfully", "success", 2200);
-        } 
+        }
         else if (response.status === "no_data") {
             toggleDataVisibility(false);
             $('#default-message').html('<h3>No data available for the selected options.</h3>');
@@ -238,7 +235,6 @@ <h3>Generated on: {{time}} </h3>
         }
     }, 'json')
     .fail(function(jqXHR, textStatus, errorThrown) {
-        console.error('Error:', errorThrown);
         toggleDataVisibility(false);
         bootstrapAlert("An error occurred while processing your request", "danger", 2200);
     });
@@ -269,4 +265,4 @@ <h3>Generated on: {{time}} </h3>
       }
   });
 </script>
-{% end %}
+{% end %}