Skip to content

Commit

Permalink
some improvements (#3445)
Browse files Browse the repository at this point in the history
* some changes

* rm circular import

* AMPLICON

* AMPLICON tests

* reservation

* fix errors

* fix qiita_db tests

* fix trace

* addressing @charles-cowart comments
  • Loading branch information
antgonza authored Dec 11, 2024
1 parent edd3eed commit ea0a7ec
Show file tree
Hide file tree
Showing 16 changed files with 84 additions and 53 deletions.
16 changes: 16 additions & 0 deletions qiita_db/analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions qiita_db/handlers/tests/test_processing_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
16 changes: 15 additions & 1 deletion qiita_db/metadata_template/prep_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions qiita_db/metadata_template/test/test_prep_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
12 changes: 7 additions & 5 deletions qiita_db/processing_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
7 changes: 7 additions & 0 deletions qiita_db/support_files/patches/93.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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_]*$' );
2 changes: 1 addition & 1 deletion qiita_db/support_files/populate_test_db.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion qiita_db/test/test_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions qiita_db/test/test_artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion qiita_db/test/test_ontology.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions qiita_db/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions qiita_pet/handlers/api_proxy/tests/test_prep_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand All @@ -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,
Expand Down
24 changes: 10 additions & 14 deletions qiita_pet/templates/resources.html
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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';
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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);

Expand Down Expand Up @@ -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>');
Expand All @@ -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);
});
Expand Down Expand Up @@ -269,4 +265,4 @@ <h3>Generated on: {{time}} </h3>
}
});
</script>
{% end %}
{% end %}
Loading

0 comments on commit ea0a7ec

Please sign in to comment.