Fixing job and reference REST API

qiita_db/handlers/oauth2.py

@@ -128,24 +128,25 @@ def write_error(self, status_code, **kwargs):
         This function is automatically called by the tornado package on errors,
         and should never be called directly.
         """
-        if status_code in {403, 404, 405}:
-            # We don't need to log these failues in the logging table
-            return
-        # log the error
         exc_info = kwargs['exc_info']
-        error_lines = ['%s\n' % line for line in format_exception(*exc_info)]
-        trace_info = ''.join(error_lines)
-        req_dict = self.request.__dict__
-        # must trim body to 1024 chars to prevent huge error messages
-        req_dict['body'] = req_dict.get('body', '')[:1024]
-        request_info = ''.join(['<strong>%s</strong>: %s\n' %
-                               (k, req_dict[k]) for k in
-                                req_dict.keys() if k != 'files'])
-        error = exc_info[1]
-        qdb.logger.LogEntry.create(
-            'Runtime',
-            'ERROR:\n%s\nTRACE:\n%s\nHTTP INFO:\n%s\n' %
-            (error, trace_info, request_info))
+
+        # We don't need to log 403, 404 or 405 failures in the logging table
+        if status_code not in {403, 404, 405}:
+            # log the error
+            error_lines = ['%s\n' % line
+                           for line in format_exception(*exc_info)]
+            trace_info = ''.join(error_lines)
+            req_dict = self.request.__dict__
+            # must trim body to 1024 chars to prevent huge error messages
+            req_dict['body'] = req_dict.get('body', '')[:1024]
+            request_info = ''.join(['<strong>%s</strong>: %s\n' %
+                                   (k, req_dict[k]) for k in
+                                    req_dict.keys() if k != 'files'])
+            error = exc_info[1]
+            qdb.logger.LogEntry.create(
+                'Runtime',
+                'ERROR:\n%s\nTRACE:\n%s\nHTTP INFO:\n%s\n' %
+                (error, trace_info, request_info))
 
         self.finish(exc_info[1].log_message)
 

qiita_db/handlers/processing_job.py

@@ -8,6 +8,8 @@
 
 from json import loads
 
+from tornado.web import HTTPError
+
 import qiita_db as qdb
 from .oauth2 import OauthBaseHandler, authenticate_oauth
 
@@ -22,20 +24,25 @@ def _get_job(job_id):
 
     Returns
     -------
-    qiita_db.processing_job.ProcessingJob, bool, string
-        The requested job or None
-        Whether if we could get the job or not
-        Error message in case we couldn't get the job
+    qiita_db.processing_job.ProcessingJob
+        The requested job
+
+    Raises
+    ------
+    HTTPError
+        If the job does not exist, with error code 404
+        If there is a problem instantiating the processing job, with error
+        code 500
     """
     if not qdb.processing_job.ProcessingJob.exists(job_id):
-        return None, False, 'Job does not exist'
+        raise HTTPError(404)
 
     try:
         job = qdb.processing_job.ProcessingJob(job_id)
-    except qdb.exceptions.QiitaDBError as e:
-        return None, False, 'Error instantiating the job: %s' % str(e)
+    except Exception as e:
+        raise HTTPError(500, 'Error instantiating the job: %s' % str(e))
 
-    return job, True, ''
+    return job
 
 
 class JobHandler(OauthBaseHandler):
@@ -51,32 +58,21 @@ def get(self, job_id):
         Returns
         -------
         dict
-            Format:
-            {'success': bool,
-             'error': str,
-             'command': str,
+            {'command': str,
              'parameters': dict of {str, obj},
              'status': str}
-
-             - success: whether the request is successful or not
-             - error: in case that success is false, it contains the error msg
              - command: the name of the command that the job executes
              - parameters: the parameters of the command, keyed by parameter
              name
              - status: the status of the job
         """
         with qdb.sql_connection.TRN:
-            job, success, error_msg = _get_job(job_id)
-            cmd = None
-            params = None
-            status = None
-            if success:
-                cmd = job.command.name
-                params = job.parameters.values
-                status = job.status
-
-        response = {'success': success, 'error': error_msg,
-                    'command': cmd, 'parameters': params,
+            job = _get_job(job_id)
+            cmd = job.command.name
+            params = job.parameters.values
+            status = job.status
+
+        response = {'command': cmd, 'parameters': params,
                     'status': status}
         self.write(response)
 
@@ -90,27 +86,16 @@ def post(self, job_id):
         ----------
         job_id : str
             The job id
-
-        Returns
-        -------
-        dict
-            Format:
-            {'success': bool,
-             'error': str}
-            - success: whether the heartbeat was successful
-            - error: in case that success is false, it contains the error msg
         """
         with qdb.sql_connection.TRN:
-            job, success, error_msg = _get_job(job_id)
-            if success:
-                try:
-                    job.update_heartbeat_state()
-                except qdb.exceptions.QiitaDBOperationNotPermittedError as e:
-                    success = False
-                    error_msg = str(e)
-
-        response = {'success': success, 'error': error_msg}
-        self.write(response)
+            job = _get_job(job_id)
+
+            try:
+                job.update_heartbeat_state()
+            except qdb.exceptions.QiitaDBOperationNotPermittedError as e:
+                raise HTTPError(403, str(e))
+
+        self.finish()
 
 
 class ActiveStepHandler(OauthBaseHandler):
@@ -122,30 +107,18 @@ def post(self, job_id):
         ----------
         job_id : str
             The job id
-
-        Returns
-        -------
-        dict
-            Format:
-            {'success': bool,
-             'error': str}
-            - success: whether the job's step was successfully updated
-            - error: in case that success is false, it contains the error msg
         """
         with qdb.sql_connection.TRN:
-            job, success, error_msg = _get_job(job_id)
-            if success:
-                job_status = job.status
-                if job_status != 'running':
-                    success = False
-                    error_msg = 'Job in a non-running state'
-                else:
-                    payload = loads(self.request.body)
-                    step = payload['step']
-                    job.step = step
-
-        response = {'success': success, 'error': error_msg}
-        self.write(response)
+            job = _get_job(job_id)
+            job_status = job.status
+            if job_status != 'running':
+                raise HTTPError(403, 'Job in a non-running state')
+            else:
+                payload = loads(self.request.body)
+                step = payload['step']
+                job.step = step
+
+        self.finish()
 
 
 class CompleteHandler(OauthBaseHandler):
@@ -157,32 +130,21 @@ def post(self, job_id):
         ----------
         job_id : str
             The job to complete
-
-        Returns
-        -------
-        dict
-            Format:
-            {'success': bool,
-             'error': str}
-            - success: whether the job information was successfuly updated
-            - error: in case that success is false, it contains the error msg
         """
         with qdb.sql_connection.TRN:
-            job, success, error_msg = _get_job(job_id)
-            if success:
-                payload = loads(self.request.body)
-                payload_success = payload['success']
-                if payload_success:
-                    artifacts = payload['artifacts']
-                    error = None
-                else:
-                    artifacts = None
-                    error = payload['error']
-                try:
-                    job.complete(payload_success, artifacts, error)
-                except qdb.exceptions.QiitaDBOperationNotPermittedError as e:
-                    success = False
-                    error_msg = str(e)
-
-        response = {'success': success, 'error': error_msg}
-        self.write(response)
+            job = _get_job(job_id)
+
+            payload = loads(self.request.body)
+            payload_success = payload['success']
+            if payload_success:
+                artifacts = payload['artifacts']
+                error = None
+            else:
+                artifacts = None
+                error = payload['error']
+            try:
+                job.complete(payload_success, artifacts, error)
+            except qdb.exceptions.QiitaDBOperationNotPermittedError as e:
+                raise HTTPError(403, str(e))
+
+        self.finish()

qiita_db/handlers/reference.py

@@ -6,6 +6,8 @@
 # The full license is in the file LICENSE, distributed with this software.
 # -----------------------------------------------------------------------------
 
+from tornado.web import HTTPError
+
 from .oauth2 import OauthBaseHandler, authenticate_oauth
 import qiita_db as qdb
 
@@ -20,19 +22,23 @@ def _get_reference(r_id):
 
     Returns
     -------
-    qiita_db.reference.Reference, bool, string
-        The requested reference or None
-        Whether if we could get the reference object or not
-        Error message in case we counldn't get the reference object
+    qiita_db.reference.Reference
+        The requested reference
+
+    Raises
+    ------
+    HTTPError
+        If the reference does not exist, with error code 404
+        If there is a problem instantiating the reference, with error code 500
     """
     try:
         reference = qdb.reference.Reference(r_id)
     except qdb.exceptions.QiitaDBUnknownIDError:
-        return None, False, 'Reference does not exist'
-    except qdb.exceptions.QiitaDBError as e:
-        return None, False, 'Error instantiating the reference: %s' % str(e)
+        raise HTTPError(404)
+    except Exception as e:
+        raise HTTPError(500, 'Error instantiating the reference: %s' % str(e))
 
-    return reference, True, ''
+    return reference
 
 
 class ReferenceFilepathsHandler(OauthBaseHandler):
@@ -49,28 +55,20 @@ def get(self, reference_id):
         Returns
         -------
         dict
-            Format:
-            {'success': bool,
-             'error': str,
-             'filepaths': list of (str, str)}
-            - success: whether the request is successful or not
-            - error: in case that success is false, it contains the error msg
-            - filepaths: the filepaths attached to the reference and their
-            filepath types
+            {'filepaths': list of (str, str)}
+            The filepaths attached to the reference and their filepath types
         """
         with qdb.sql_connection.TRN:
-            reference, success, error_msg = _get_reference(reference_id)
-            fps = None
-            if success:
-                fps = [(reference.sequence_fp, "reference_seqs")]
-                tax_fp = reference.taxonomy_fp
-                if tax_fp:
-                    fps.append((tax_fp, "reference_tax"))
-                tree_fp = reference.tree_fp
-                if tree_fp:
-                    fps.append((tree_fp, "reference_tree"))
+            reference = _get_reference(reference_id)
+
+            fps = [(reference.sequence_fp, "reference_seqs")]
+            tax_fp = reference.taxonomy_fp
+            if tax_fp:
+                fps.append((tax_fp, "reference_tax"))
+            tree_fp = reference.tree_fp
+            if tree_fp:
+                fps.append((tree_fp, "reference_tree"))
 
-            response = {'success': success, 'error': error_msg,
-                        'filepaths': fps}
+            response = {'filepaths': fps}
 
         self.write(response)