From 04c0c71f38c59545ff9f36abb1c815e552195aff Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Mon, 3 Jul 2017 09:27:34 -0700 Subject: [PATCH 1/4] Fixing race condition --- qiita_pet/handlers/analysis_handlers/base_handlers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qiita_pet/handlers/analysis_handlers/base_handlers.py b/qiita_pet/handlers/analysis_handlers/base_handlers.py index f69eda5be..9f6a445d6 100644 --- a/qiita_pet/handlers/analysis_handlers/base_handlers.py +++ b/qiita_pet/handlers/analysis_handlers/base_handlers.py @@ -21,7 +21,6 @@ class CreateAnalysisHandler(BaseHandler): @authenticated - @execute_as_transaction def post(self): name = self.get_argument('name') desc = self.get_argument('description') From c1da08f83b94372b572aede545e0f575279a43c7 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Mon, 3 Jul 2017 10:15:24 -0700 Subject: [PATCH 2/4] Removing unused code and fixing typo --- qiita_pet/handlers/api_proxy/__init__.py | 6 +- qiita_pet/handlers/api_proxy/artifact.py | 190 +---------------- .../handlers/api_proxy/tests/test_artifact.py | 196 +----------------- qiita_pet/handlers/study_handlers/__init__.py | 5 +- qiita_pet/handlers/study_handlers/artifact.py | 21 +- .../artifact_ajax/artifact_summary.html | 2 +- .../study_ajax/artifact_summary.html | 196 ------------------ 7 files changed, 9 insertions(+), 607 deletions(-) delete mode 100644 qiita_pet/templates/study_ajax/artifact_summary.html diff --git a/qiita_pet/handlers/api_proxy/__init__.py b/qiita_pet/handlers/api_proxy/__init__.py index 670365075..9dee67bb5 100644 --- a/qiita_pet/handlers/api_proxy/__init__.py +++ b/qiita_pet/handlers/api_proxy/__init__.py @@ -30,9 +30,8 @@ study_tags_request) from .artifact import (artifact_graph_get_req, artifact_types_get_req, artifact_post_req, artifact_get_req, - artifact_status_put_req, - artifact_summary_get_request, artifact_get_prep_req, - artifact_summary_post_request, artifact_patch_request) + artifact_status_put_req, artifact_get_prep_req, + artifact_patch_request) from .ontology import ontology_patch_handler from .processing import ( list_commands_handler_get_req, process_artifact_handler_get_req, @@ -62,7 +61,6 @@ 'study_tags_request', 'study_tags_patch_request', 'study_get_tags_request', 'prep_template_patch_req', 'ontology_patch_handler', - 'artifact_summary_get_request', 'artifact_summary_post_request', 'list_commands_handler_get_req', 'process_artifact_handler_get_req', 'list_options_handler_get_req', 'workflow_handler_post_req', 'workflow_handler_patch_req', 'workflow_run_post_req', diff --git a/qiita_pet/handlers/api_proxy/artifact.py b/qiita_pet/handlers/api_proxy/artifact.py index b47f9c040..2d8e10b51 100644 --- a/qiita_pet/handlers/api_proxy/artifact.py +++ b/qiita_pet/handlers/api_proxy/artifact.py @@ -5,7 +5,7 @@ # # The full license is in the file LICENSE, distributed with this software. # ----------------------------------------------------------------------------- -from os.path import join, basename +from os.path import join from functools import partial from json import dumps @@ -28,194 +28,6 @@ PREP_TEMPLATE_KEY_FORMAT = 'prep_template_%s' -def artifact_summary_get_request(user_id, artifact_id): - """Returns the information for the artifact summary page - - Parameters - ---------- - user_id : str - The user making the request - artifact_id : int or str - The artifact id - - Returns - ------- - dict of objects - A dictionary containing the artifact summary information - {'status': str, - 'message': str, - 'name': str, - 'summary': str, - 'job': list of [str, str, str]} - """ - artifact_id = int(artifact_id) - artifact = Artifact(artifact_id) - - access_error = check_access(artifact.study.id, user_id) - if access_error: - return access_error - - user = User(user_id) - visibility = artifact.visibility - summary = artifact.html_summary_fp - job_info = None - errored_jobs = [] - processing_jobs = [] - for j in artifact.jobs(): - if j.command.software.type == "artifact transformation": - status = j.status - if status == 'success': - continue - j_msg = j.log.msg if status == 'error' else None - processing_jobs.append( - [j.id, j.command.name, j.status, j.step, j_msg]) - - # Check if the HTML summary exists - if summary: - with open(summary[1]) as f: - summary = f.read() - else: - # Check if the summary is being generated - command = Command.get_html_generator(artifact.artifact_type) - all_jobs = set(artifact.jobs(cmd=command)) - jobs = [j for j in all_jobs if j.status in ['queued', 'running']] - errored_jobs = [(j.id, j.log.msg) - for j in all_jobs if j.status in ['error']] - if jobs: - # There is already a job generating the HTML. Also, there should be - # at most one job, because we are not allowing here to start more - # than one - job = jobs[0] - job_info = [job.id, job.status, job.step] - - buttons = [] - btn_base = ( - '').format(artifact_id) - - if qiita_config.require_approval: - if visibility == 'sandbox': - # The request approval button only appears if the artifact is - # sandboxed and the qiita_config specifies that the approval should - # be requested - buttons.append( - btn_base % ('request approval for', 'awaiting_approval', - 'Request approval')) - - elif user.level == 'admin' and visibility == 'awaiting_approval': - # The approve artifact button only appears if the user is an admin - # the artifact is waiting to be approvaed and the qiita config - # requires artifact approval - buttons.append(btn_base % ('approve', 'private', - 'Approve artifact')) - - if visibility == 'private': - # The make public button only appears if the artifact is private - buttons.append(btn_base % ('make public', 'public', 'Make public')) - - # The revert to sandbox button only appears if the artifact is not - # sandboxed nor public - if visibility not in {'sandbox', 'public'}: - buttons.append(btn_base % ('revert to sandbox', 'sandbox', - 'Revert to sandbox')) - - if user.level == 'admin': - if artifact.can_be_submitted_to_ebi: - if not artifact.is_submitted_to_ebi: - buttons.append( - '' - '' - ' Submit to EBI' % artifact_id) - if artifact.can_be_submitted_to_vamps: - if not artifact.is_submitted_to_vamps: - buttons.append( - '' - '' - ' Submit to VAMPS' % artifact_id) - - files = [(f_id, "%s (%s)" % (basename(fp), f_type.replace('_', ' '))) - for f_id, fp, f_type in artifact.filepaths - if f_type != 'directory'] - - # TODO: https://github.com/biocore/qiita/issues/1724 Remove this hardcoded - # values to actually get the information from the database once it stores - # the information - if artifact.artifact_type in ['SFF', 'FASTQ', 'FASTA', 'FASTA_Sanger', - 'per_sample_FASTQ']: - # If the artifact is one of the "raw" types, only the owner of the - # study and users that has been shared with can see the files - if not artifact.study.has_access(user, no_public=True): - files = [] - - processing_parameters = (artifact.processing_parameters.values - if artifact.processing_parameters is not None - else {}) - - return {'status': 'success', - 'message': '', - 'name': artifact.name, - 'summary': summary, - 'job': job_info, - 'errored_jobs': errored_jobs, - 'processing_jobs': processing_jobs, - 'visibility': visibility, - 'buttons': ' '.join(buttons), - 'files': files, - 'editable': artifact.study.can_edit(user), - 'study_id': artifact.study.id, - 'prep_id': artifact.prep_templates[0].id, - 'processing_parameters': processing_parameters} - - -def artifact_summary_post_request(user_id, artifact_id): - """Launches the HTML summary generation and returns the job information - - Parameters - ---------- - user_id : str - The user making the request - artifact_id : int or str - The artifact id - - Returns - ------- - dict of objects - A dictionary containing the artifact summary information - {'status': str, - 'message': str, - 'job': list of [str, str, str]} - """ - artifact_id = int(artifact_id) - artifact = Artifact(artifact_id) - - access_error = check_access(artifact.study.id, user_id) - if access_error: - return access_error - - # Check if the summary is being generated or has been already generated - command = Command.get_html_generator(artifact.artifact_type) - jobs = artifact.jobs(cmd=command) - jobs = [j for j in jobs if j.status in ['queued', 'running', 'success']] - if jobs: - # The HTML summary is either being generated or already generated. - # Return the information of that job so we only generate the HTML - # once - job = jobs[0] - else: - # Create a new job to generate the HTML summary and return the newly - # created job information - job = ProcessingJob.create( - User(user_id), - Parameters.load(command, values_dict={'input_data': artifact_id})) - job.submit() - - return {'status': 'success', - 'message': '', - 'job': [job.id, job.status, job.step]} - - def artifact_get_req(user_id, artifact_id): """Returns all base information about an artifact diff --git a/qiita_pet/handlers/api_proxy/tests/test_artifact.py b/qiita_pet/handlers/api_proxy/tests/test_artifact.py index e6e115446..a4e92cfff 100644 --- a/qiita_pet/handlers/api_proxy/tests/test_artifact.py +++ b/qiita_pet/handlers/api_proxy/tests/test_artifact.py @@ -6,7 +6,7 @@ # The full license is in the file LICENSE, distributed with this software. # ----------------------------------------------------------------------------- from unittest import TestCase, main -from os.path import join, exists, basename +from os.path import join, exists from os import remove, close from datetime import datetime from tempfile import mkstemp @@ -21,14 +21,11 @@ from qiita_db.metadata_template.prep_template import PrepTemplate from qiita_db.study import Study from qiita_db.util import get_mountpoint -from qiita_db.processing_job import ProcessingJob -from qiita_db.user import User -from qiita_db.software import Command, Parameters, DefaultParameters +from qiita_db.software import Parameters, DefaultParameters from qiita_db.exceptions import QiitaDBWarning from qiita_pet.handlers.api_proxy.artifact import ( artifact_get_req, artifact_status_put_req, artifact_graph_get_req, artifact_types_get_req, artifact_post_req, - artifact_summary_get_request, artifact_summary_post_request, artifact_patch_request, artifact_get_prep_req) @@ -161,195 +158,6 @@ def tearDown(self): r_client.flushdb() - def test_artifact_summary_get_request(self): - # Artifact w/o summary - obs = artifact_summary_get_request('test@foo.bar', 1) - exp_p_jobs = [ - ['063e553b-327c-4818-ab4a-adfe58e49860', 'Split libraries FASTQ', - 'queued', None, None], - ['bcc7ebcd-39c1-43e4-af2d-822e3589f14d', 'Split libraries', - 'running', 'demultiplexing', None]] - exp_files = [ - (1L, '1_s_G1_L001_sequences.fastq.gz (raw forward seqs)'), - (2L, '1_s_G1_L001_sequences_barcodes.fastq.gz (raw barcodes)')] - exp = {'status': 'success', - 'message': '', - 'name': 'Raw data 1', - 'processing_parameters': {}, - 'summary': None, - 'job': None, - 'processing_jobs': exp_p_jobs, - 'errored_jobs': [], - 'visibility': 'private', - 'buttons': (' '), - 'files': exp_files, - 'editable': True, - 'prep_id': 1, - 'study_id': 1} - self.assertEqual(obs, exp) - - # Artifact with summary being generated - job = ProcessingJob.create( - User('test@foo.bar'), - Parameters.load(Command(7), values_dict={'input_data': 1}) - ) - job._set_status('queued') - obs = artifact_summary_get_request('test@foo.bar', 1) - exp = {'status': 'success', - 'message': '', - 'name': 'Raw data 1', - 'processing_parameters': {}, - 'summary': None, - 'job': [job.id, 'queued', None], - 'processing_jobs': exp_p_jobs, - 'errored_jobs': [], - 'visibility': 'private', - 'buttons': (' '), - 'files': exp_files, - 'editable': True, - 'prep_id': 1, - 'study_id': 1} - self.assertEqual(obs, exp) - - # Artifact with summary - fd, fp = mkstemp(suffix=".html") - close(fd) - with open(fp, 'w') as f: - f.write('HTML TEST - not important\n') - a = Artifact(1) - a.set_html_summary(fp) - self._files_to_remove.extend([fp, a.html_summary_fp[1]]) - exp_files.append( - (a.html_summary_fp[0], - '%s (html summary)' % basename(a.html_summary_fp[1]))) - obs = artifact_summary_get_request('test@foo.bar', 1) - exp = {'status': 'success', - 'message': '', - 'name': 'Raw data 1', - 'processing_parameters': {}, - 'summary': 'HTML TEST - not important\n', - 'job': None, - 'processing_jobs': exp_p_jobs, - 'errored_jobs': [], - 'visibility': 'private', - 'buttons': (' '), - 'files': exp_files, - 'editable': True, - 'prep_id': 1, - 'study_id': 1} - self.assertEqual(obs, exp) - - # No access - obs = artifact_summary_get_request('demo@microbio.me', 1) - exp = {'status': 'error', - 'message': 'User does not have access to study'} - self.assertEqual(obs, exp) - - # A non-owner/share user can't see the files - a.visibility = 'public' - obs = artifact_summary_get_request('demo@microbio.me', 1) - exp = {'status': 'success', - 'message': '', - 'name': 'Raw data 1', - 'processing_parameters': {}, - 'summary': 'HTML TEST - not important\n', - 'job': None, - 'processing_jobs': exp_p_jobs, - 'errored_jobs': [], - 'visibility': 'public', - 'buttons': '', - 'files': [], - 'editable': False, - 'prep_id': 1, - 'study_id': 1} - self.assertEqual(obs, exp) - - # returnig to private - a.visibility = 'sandbox' - - # admin gets buttons - obs = artifact_summary_get_request('admin@foo.bar', 2) - exp_p_jobs = [ - ['d19f76ee-274e-4c1b-b3a2-a12d73507c55', - 'Pick closed-reference OTUs', 'error', 'generating demux file', - 'Error message']] - exp_files = [ - (3L, '1_seqs.fna (preprocessed fasta)'), - (4L, '1_seqs.qual (preprocessed fastq)'), - (5L, '1_seqs.demux (preprocessed demux)')] - exp = {'status': 'success', - 'files': exp_files, - 'errored_jobs': [], - 'editable': True, - 'visibility': 'sandbox', - 'job': None, - 'message': '', - 'name': 'Demultiplexed 1', - 'processing_jobs': exp_p_jobs, - 'processing_parameters': { - 'max_barcode_errors': 1.5, 'sequence_max_n': 0, - 'max_bad_run_length': 3, 'phred_offset': u'auto', - 'rev_comp': False, 'phred_quality_threshold': 3, - 'input_data': 1, 'rev_comp_barcode': False, - 'rev_comp_mapping_barcodes': False, - 'min_per_read_length_fraction': 0.75, - 'barcode_type': u'golay_12'}, - 'summary': None, - 'buttons': ( - ' ' - ' Submit to ' - 'VAMPS'), - 'study_id': 1, - 'prep_id': 1} - self.assertEqual(obs, exp) - - def test_artifact_summary_post_request(self): - # No access - obs = artifact_summary_post_request('demo@microbio.me', 1) - exp = {'status': 'error', - 'message': 'User does not have access to study'} - self.assertEqual(obs, exp) - - # Returns already existing job - job = ProcessingJob.create( - User('test@foo.bar'), - Parameters.load(Command(7), values_dict={'input_data': 2}) - ) - job._set_status('queued') - obs = artifact_summary_post_request('test@foo.bar', 2) - exp = {'status': 'success', - 'message': '', - 'job': [job.id, 'queued', None]} - self.assertEqual(obs, exp) - def test_artifact_patch_request(self): obs = artifact_patch_request('test@foo.bar', 'replace', '/%d/name/' % self.artifact.id, diff --git a/qiita_pet/handlers/study_handlers/__init__.py b/qiita_pet/handlers/study_handlers/__init__.py index 0cd6011d3..ed7f47c4d 100644 --- a/qiita_pet/handlers/study_handlers/__init__.py +++ b/qiita_pet/handlers/study_handlers/__init__.py @@ -21,8 +21,7 @@ ListOptionsHandler, WorkflowHandler, WorkflowRunHandler, JobAJAX) from .artifact import (ArtifactGraphAJAX, NewArtifactHandler, - ArtifactAdminAJAX, ArtifactSummaryAJAX, - ArtifactGetSamples) + ArtifactAdminAJAX, ArtifactGetSamples) from .sample_template import SampleTemplateAJAX, SampleAJAX __all__ = ['ListStudiesHandler', 'StudyApprovalList', 'ShareStudyAJAX', @@ -34,6 +33,6 @@ 'ListCommandsHandler', 'ListOptionsHandler', 'SampleAJAX', 'StudyDeleteAjax', 'NewPrepTemplateAjax', 'DataTypesMenuAJAX', 'StudyFilesAJAX', 'PrepTemplateSummaryAJAX', - 'ArtifactSummaryAJAX', 'WorkflowHandler', 'WorkflowRunHandler', + 'WorkflowHandler', 'WorkflowRunHandler', 'JobAJAX', 'AutocompleteHandler', 'StudyGetTags', 'StudyTags', 'ArtifactGetSamples'] diff --git a/qiita_pet/handlers/study_handlers/artifact.py b/qiita_pet/handlers/study_handlers/artifact.py index 9501c65fe..cae160e8c 100644 --- a/qiita_pet/handlers/study_handlers/artifact.py +++ b/qiita_pet/handlers/study_handlers/artifact.py @@ -14,9 +14,7 @@ from qiita_pet.handlers.base_handlers import BaseHandler from qiita_pet.handlers.api_proxy import ( artifact_graph_get_req, artifact_types_get_req, artifact_post_req, - artifact_status_put_req, artifact_get_req, - artifact_summary_get_request, artifact_summary_post_request, - artifact_get_prep_req) + artifact_status_put_req, artifact_get_req, artifact_get_prep_req) from qiita_core.util import execute_as_transaction from qiita_core.qiita_settings import qiita_config @@ -60,23 +58,6 @@ def post(self): self.write(artifact) -class ArtifactSummaryAJAX(BaseHandler): - @authenticated - def get(self): - artifact_id = self.get_argument('artifact_id') - user_id = self.current_user.id - res = artifact_summary_get_request(user_id, artifact_id) - res['artifact_id'] = artifact_id - res['user_id'] = user_id - self.render("study_ajax/artifact_summary.html", **res) - - @authenticated - def post(self): - artifact_id = self.get_argument('artifact_id') - res = artifact_summary_post_request(self.current_user.id, artifact_id) - self.write(res) - - class ArtifactGetSamples(BaseHandler): @authenticated def get(self): diff --git a/qiita_pet/templates/artifact_ajax/artifact_summary.html b/qiita_pet/templates/artifact_ajax/artifact_summary.html index 0162d7df1..32ee96484 100644 --- a/qiita_pet/templates/artifact_ajax/artifact_summary.html +++ b/qiita_pet/templates/artifact_ajax/artifact_summary.html @@ -135,7 +135,7 @@

Step: {{j_step}} {% end %} {% if j_error %} - Erro message: {{j_error}} + Error message: {{j_error}} {% end %}
{% end %} diff --git a/qiita_pet/templates/study_ajax/artifact_summary.html b/qiita_pet/templates/study_ajax/artifact_summary.html deleted file mode 100644 index 55a98ecf5..000000000 --- a/qiita_pet/templates/study_ajax/artifact_summary.html +++ /dev/null @@ -1,196 +0,0 @@ -{% from qiita_core.qiita_settings import qiita_config %} - -
-
-

- {{name}} (ID: {{artifact_id}}) - {% if editable %} - Edit - Process - Delete - {% end %} -

- - Processing parameters: - {% for key in processing_parameters %} - {%raw key%}: {%raw processing_parameters[key]%} - {% end %} -

- -
-
-
-
- Visibility: {{visibility}} - {% if editable %} - {% raw buttons %} - {% end %} -
-
- -{% if files %} -
-
- Available files: - {% for f_id, f_name in files %} -
{{f_name}} - {% end %} -
-
-{% end %} - -
-
- {% if processing_jobs %} - Jobs using this set of files:
- {% end %} - {% for j_id, c_name, j_status, j_step, j_error in processing_jobs %} - Job {{j_id}} ({{c_name}}). Status: {{j_status}}. - {% if j_step %} - Step: {{j_step}} - {% end %} - {% if j_error %} - Erro message: {{j_error}} - {% end %} -
- {% end %} -
-
-
-
- {% if summary is not None %} - {% raw summary %} - {% elif job is not None %} - Job {{ job[0] }}: {{ job[1] }} {{ job[2] }} - {% else %} - Currently, no summary exists.
-
- {% for j_id, j_error in errored_jobs %} - Job {{ j_id }}: {% raw j_error.replace('\n', '
') %}
- {% end %} - {% end %} -
-
- - - From 12ed1c024c8a269b61fae0a61b08c4b32f61047e Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Mon, 3 Jul 2017 10:23:03 -0700 Subject: [PATCH 3/4] Hiding job step if success --- qiita_pet/templates/analysis_description.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiita_pet/templates/analysis_description.html b/qiita_pet/templates/analysis_description.html index 765c8c4e3..0c991eec5 100644 --- a/qiita_pet/templates/analysis_description.html +++ b/qiita_pet/templates/analysis_description.html @@ -57,7 +57,7 @@ function createJobHTML(data, target) { target = "#" + target; $(target).empty(); - var keys = ['job_id', 'job_status', 'job_step']; + var keys = ['job_id', 'job_status']; var d = $("
").appendTo(target); for(var i in keys) { if (data[keys[i]]) { From 5d74f8bfda36bf29ebb1f106f616fe3f11c72e67 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Mon, 3 Jul 2017 11:20:41 -0700 Subject: [PATCH 4/4] Showing loading gif and fixing typo --- qiita_pet/templates/artifact_ajax/processing_artifact.html | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qiita_pet/templates/artifact_ajax/processing_artifact.html b/qiita_pet/templates/artifact_ajax/processing_artifact.html index 415d0b00c..e970b0a70 100644 --- a/qiita_pet/templates/artifact_ajax/processing_artifact.html +++ b/qiita_pet/templates/artifact_ajax/processing_artifact.html @@ -129,7 +129,7 @@ type: 'PATCH', data: {'op': 'add', 'path': workflow_id, 'value': JSON.stringify(value)}, success: function(data) { - if(data.stats == 'error') { + if(data.status == 'error') { bootstrapAlert(data.message, "danger"); } else { @@ -177,6 +177,9 @@ * */ function add_job() { + show_loading('graph-network-div'); + show_loading('processing-info-div'); + var command_id = $("#command-sel").val(); var params_id = $("#params-sel").val(); var params = {};