From 0ffe1c57d97566946def4bc548fc039809d6423c Mon Sep 17 00:00:00 2001 From: totycro Date: Wed, 23 Dec 2020 17:27:58 +0100 Subject: [PATCH] Minor improvements and cleanup of process API (#594) * Move tinydb configuration from base manager to tinydb * Rename job_result to job It's actually the job metadata * Make get_job_result only return the result The status is part of the metadata which is already returned by get_job * Detect format from request headers, not response headers * Default to json output, not html This is a reasonable default because mostly browsers want html, and they always request html, whereas dev tools used by api users might not always request json. * Don't provide outputs when executing async an job In this case, we don't have any outputs yet. * Make connection an optional attribute for managers instead of only defining it for tinydb --- pygeoapi/api.py | 32 ++++++++++++++++------------- pygeoapi/process/manager/base.py | 2 +- pygeoapi/process/manager/tinydb_.py | 10 ++++----- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/pygeoapi/api.py b/pygeoapi/api.py index 90586d9..ab728e7 100644 --- a/pygeoapi/api.py +++ b/pygeoapi/api.py @@ -2128,7 +2128,7 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' else: response = outputs - elif status != JobStatus.failed: + elif status != JobStatus.failed and not is_async: response['outputs'] = outputs if is_async: @@ -2161,9 +2161,9 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' LOGGER.info(exception) return headers_, 404, to_json(exception, self.pretty_print) - job_result = self.manager.get_job(process_id, job_id) + job = self.manager.get_job(process_id, job_id) - if not job_result: + if not job: exception = { 'code': 'NoSuchJob', 'description': 'job not found' @@ -2171,7 +2171,7 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' LOGGER.info(exception) return headers_, 404, to_json(exception, self.pretty_print) - format_ = check_format(args, headers_) + format_ = check_format(args, headers) if format_ is not None and format_ not in FORMATS: exception = { 'code': 'InvalidParameterValue', @@ -2180,11 +2180,11 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' LOGGER.error(exception) return headers_, 400, to_json(exception, self.pretty_print) - status = JobStatus[job_result['status']] + status = JobStatus[job['status']] response = { 'jobID': job_id, 'status': status.value, - 'message': job_result.get('message', None), + 'message': job.get('message', None), 'links': [{ 'href': '{}/processes/{}/jobs/{}'.format( self.config['server']['url'], process_id, job_id @@ -2210,7 +2210,7 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' # TODO link to exception report? pass - if format_ == 'json' or format_ == 'jsonld': + if format_ != 'html': return headers_, 200, json.dumps(response, default=json_serial) else: headers_['Content-Type'] = 'text/html' @@ -2222,13 +2222,13 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' 'title': process.metadata['title'] } - psd = job_result.get('process_start_datetime', None) - ped = job_result.get('process_end_datetime', None) + psd = job.get('process_start_datetime', None) + ped = job.get('process_end_datetime', None) if status == JobStatus.successful: progress = 100 else: - progress = job_result.get('progress', 0) + progress = job.get('progress', 0) job_info = { 'process_start_datetime': psd, @@ -2276,9 +2276,9 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' LOGGER.info(exception) return headers_, 404, json.dumps(exception) - status, job_output = self.manager.get_job_result(process_id, job_id) + job = self.manager.get_job(process_id, job_id) - if not status: + if not job: exception = { 'code': 'NoSuchJob', 'description': 'job not found' @@ -2286,6 +2286,8 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' LOGGER.info(exception) return headers_, 404, json.dumps(exception) + status = JobStatus[job['status']] + if status == JobStatus.running: exception = { 'code': 'ResultNotReady', @@ -2311,9 +2313,11 @@ tiles/{{{}}}/{{{}}}/{{{}}}/{{{}}}?f=mvt' LOGGER.info(exception) return headers_, 400, json.dumps(exception) - format_ = check_format(args, headers_) + job_output = self.manager.get_job_result(process_id, job_id) - if not format_ or format_ == 'html': + format_ = check_format(args, headers) + + if format_ == 'html': headers_['Content-Type'] = 'text/html' data = { 'process': { diff --git a/pygeoapi/process/manager/base.py b/pygeoapi/process/manager/base.py index bfb426d..a3c9881 100644 --- a/pygeoapi/process/manager/base.py +++ b/pygeoapi/process/manager/base.py @@ -53,7 +53,7 @@ class BaseManager: self.name = manager_def['name'] self.is_async = False - self.connection = manager_def['connection'] + self.connection = manager_def.get('connection', None) self.output_dir = manager_def.get('output_dir', None) def get_jobs(self, process_id=None, status=None): diff --git a/pygeoapi/process/manager/tinydb_.py b/pygeoapi/process/manager/tinydb_.py index d6b8a0b..8a65131 100644 --- a/pygeoapi/process/manager/tinydb_.py +++ b/pygeoapi/process/manager/tinydb_.py @@ -179,26 +179,26 @@ class TinyDBManager(BaseManager): :param process_id: process identifier :param jobid: job identifier - :returns: tuple of JobStatus and the process output as a `dict` + :returns: The process output as a `dict` """ job_result = self.get_job(process_id, job_id) if not job_result: # processs/job does not exist - return None, None + return None location = job_result.get('location', None) job_status = JobStatus[job_result['status']] if not job_status == JobStatus.successful: # Job is incomplete - return job_status, None + return None if not location: # Job data was not written for some reason # TODO log/raise exception? - return job_status, {} + return {} with io.open(location, 'r', encoding='utf-8') as filehandler: result = json.load(filehandler) - return job_status, result + return result def __repr__(self): return ' {}'.format(self.name)