diff --git a/CHANGELOG.md b/CHANGELOG.md index 079b2e0d..4bc4aa44 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Added enhancements - Improved clean module so that an aux cleaning file is employed for specific services if applicable [#692](https://github.com/BU-ISCIII/buisciii-tools/pull/692). +- Improved HTTP error handling in drylab_api module and fixed fix-permissions so that it does not break given insufficient permissions [#694](https://github.com/BU-ISCIII/buisciii-tools/pull/694). #### Fixes diff --git a/buisciii/__main__.py b/buisciii/__main__.py index ce3dcf78..395a0ea7 100755 --- a/buisciii/__main__.py +++ b/buisciii/__main__.py @@ -998,7 +998,7 @@ def fix_permissions(ctx, input_directory): else: log.exception(f"EXCEPTION FOUND: {e}") stderr.print(f"EXCEPTION FOUND: {e}") - sys.exit(1) + continue if __name__ == "__main__": diff --git a/buisciii/drylab_api.py b/buisciii/drylab_api.py index 43915298..c8b5225c 100755 --- a/buisciii/drylab_api.py +++ b/buisciii/drylab_api.py @@ -4,6 +4,7 @@ import requests import sys import rich +from http import HTTPStatus import buisciii.utils log = logging.getLogger(__name__) @@ -34,27 +35,36 @@ def __init__(self, server, url, user, password): # TODO: this is waaay too dirty, find a way to pass variable number of parameters and values. # by Guille: I used an f-string instead of all the + stuff, I think thats cleaner? # by Guille: **kwargs time! + + def handle_error_status(self, status_code, safe, context=None): + try: + status = HTTPStatus(status_code) + message = f"{status_code} {status.phrase}: {status.description}." + except ValueError: + message = f"Unexpected HTTP status code: {status_code}." + + if context: + message += f" Context: {context}" + + if safe: + log.error(message) + stderr.print(message) + sys.exit(1) + else: + return status_code + def get_request(self, request_info, safe=True, **kwargs): url_http = f"{self.request_url}{request_info}?{''.join([f'{key}={value}&' for key,value in kwargs.items()])[:-1]}" try: req = requests.get(url_http, headers=self.headers) if req.status_code > 201: - if safe: - resolution = kwargs.get("resolution", "unknown") - log.info( - f"Resolution {resolution} does not exist. Status code: {req.status_code}" - ) - stderr.print( - f"Resolution {resolution} does not exist! Please make sure the resolution ID is correct and has been created" - ) - sys.exit(1) - else: - return req.status_code + resolution = kwargs.get("resolution", None) + context = f"resolution={resolution}" if resolution else None + return self.handle_error_status(req.status_code, safe, context) return json.loads(req.text) except requests.ConnectionError: log.error("Unable to open connection towards iSkyLIMS, aborting") sys.exit(1) - return False def put_request( self, request_info, parameter1, value1, parameter2, value2, safe=True @@ -74,15 +84,7 @@ def put_request( try: req = requests.put(url_http, headers=self.headers, auth=self.auth) if req.status_code > 201: - if safe: - log.error( - "Resolution id does not exist. Status code: " - + str(req.status_code) - ) - sys.exit(1) - else: - return req.status_code - # return json.loads(req.text) + return self.handle_error_status(req.status_code, safe) return True except requests.ConnectionError: log.error("Unable to open connection towards iSkyLIMS") @@ -96,16 +98,8 @@ def post_request(self, request_info, data, safe=True): url_http, data=data, headers=self.headers, auth=self.auth ) if req.status_code > 201: - if safe: - log.error( - "Some error occurred. Status code: " + str(req.status_code) - ) - log.error("Status text: " + str(json.loads(req.text))) - sys.exit() - else: - return req.status_code + return self.handle_error_status(req.status_code, safe) return True - except requests.ConnectionError: log.error("Unable to open connection towards iSkyLIMS, aborting") sys.exit(1)