From 8670290568376cd85bf8387b00f8c71abeb1175f Mon Sep 17 00:00:00 2001 From: Daniel K Lyons <dlyons@nrao.edu> Date: Mon, 29 Mar 2021 16:06:45 -0600 Subject: [PATCH] Remove all of the direct sys.exit calls except from main(). Refactor to raise exceptions only. --- .../datafetcher/datafetcher/datafetcher.py | 94 +++----- .../datafetcher/datafetcher/errors.py | 202 ++++++------------ .../datafetcher/locations_report.py | 6 +- .../datafetcher/project_fetcher.py | 69 ++---- .../datafetcher/datafetcher/return_codes.py | 17 -- .../datafetcher/datafetcher/utilities.py | 20 +- .../datafetcher/test/df_pytest_utils.py | 24 +-- .../datafetcher/test/mock_data_fetcher.py | 31 +-- .../datafetcher/test/test_df_function.py | 26 +-- .../datafetcher/test/test_df_return_codes.py | 46 ++-- 10 files changed, 153 insertions(+), 382 deletions(-) diff --git a/apps/cli/executables/datafetcher/datafetcher/datafetcher.py b/apps/cli/executables/datafetcher/datafetcher/datafetcher.py index ad19dbaa6..2ae9f2a10 100755 --- a/apps/cli/executables/datafetcher/datafetcher/datafetcher.py +++ b/apps/cli/executables/datafetcher/datafetcher/datafetcher.py @@ -8,9 +8,8 @@ import sys from argparse import Namespace from pathlib import Path -from datafetcher.errors import MissingSettingsException, NoProfileException +from datafetcher.errors import MissingSettingsException, NoProfileException, DataFetcherException from datafetcher.project_fetcher import ParallelFetcher -from datafetcher.return_codes import ReturnCode from .locations_report import LocationsReport from .utilities import get_arg_parser, get_capo_settings, FlexLogger, path_is_accessible @@ -48,7 +47,7 @@ class DataFetcher: def __init__(self, args: Namespace, df_capo_settings: dict): self.usage = self._build_usage_message() if args is None or df_capo_settings is None: - self._exit_with_error(ReturnCode.MISSING_SETTING) + raise MissingSettingsException() self.args = args self.settings = df_capo_settings self.verbose = self.args.verbose @@ -56,52 +55,39 @@ class DataFetcher: # required arguments self.output_dir = args.output_dir if self.output_dir is None: - self._exit_with_error(ReturnCode.MISSING_SETTING) + raise MissingSettingsException("output directory option is missing") output_dir = Path(self.output_dir) if not output_dir.is_dir() or not path_is_accessible(output_dir): - logging.error(f"output location {self.output_dir} inaccessible " f"or not found") - self._exit_with_error(ReturnCode.MISSING_SETTING) + raise MissingSettingsException( + f"output location {self.output_dir} inaccessible or not found" + ) self._LOG = FlexLogger(self.__class__.__name__, self.output_dir, self.verbose) if args.location_file is not None: if args.product_locator is not None: - self._LOG.error("required: location file OR product locator " "-- not both") - self._exit_with_error(ReturnCode.MISSING_SETTING) + raise MissingSettingsException( + "required: location file OR product locator -- not both" + ) self.location_file = args.location_file elif args.product_locator is not None: self.product_locator = args.product_locator else: - self._LOG.error("you must specify either a location file or a " "product locator") - self._exit_with_error(ReturnCode.MISSING_SETTING) + raise MissingSettingsException( + "you must specify either a location file or a product locator" + ) self.profile = args.profile if self.profile is None: - self._exit_with_error(ReturnCode.MISSING_PROFILE) + raise NoProfileException() # optional arguments self.is_dry = args.dry_run self.force = args.force self.verbose = args.verbose or False - try: - self.locations_report = self._get_locations() - self.servers_report = self.locations_report.servers_report - except SystemExit as exc: - self._LOG.error(f"{exc}") - if args.location_file: - self._exit_with_error(ReturnCode.CANNOT_OPEN_LOCATION_FILE) - else: - self._exit_with_error(ReturnCode.PRODUCT_LOCATOR_NOT_FOUND) - raise - - except Exception as exc: - self._LOG.error(f">>> throwing unexpected {type(exc)} during init: {exc}") - raise - - def _exit_with_error(self, return_code: ReturnCode): - print(self.usage) - sys.exit(return_code.value["code"]) + self.locations_report = self._get_locations() + self.servers_report = self.locations_report.servers_report @staticmethod def _build_usage_message() -> str: @@ -110,9 +96,6 @@ class DataFetcher: (--product-locator PRODUCT_LOCATOR | --location-file LOCATION_FILE) [--dry-run] [--output-dir OUTPUT_DIR] [-v, --verbose] [--profile PROFILE]\n""" - usage_str += "\n\t\tReturn codes:\n" - for return_code in ReturnCode: - usage_str += f"\n\t\t\t{return_code.value['code']}: " f"{return_code.value['text']}" return usage_str def run(self): @@ -121,36 +104,12 @@ class DataFetcher: :return: """ - try: - fetcher = ParallelFetcher(self.args, self.settings, self._LOG, self.servers_report) - return fetcher.run() - except SystemExit as exc: - self._LOG.error(f"{exc}") - raise - except Exception as exc: - self._LOG.error(f">>> throwing unexpected exception during run: {exc}") - raise + fetcher = ParallelFetcher(self.args, self.settings, self._LOG, self.servers_report) + return fetcher.run() def _get_locations(self): - try: - capo_settings = get_capo_settings(self.profile) - except MissingSettingsException as exc: - # if a setting couldn't be found, we can be pretty sure that - # the profile's no good - raise NoProfileException from exc - - try: - return LocationsReport(self._LOG, self.args, capo_settings) - - except Exception as exc: - self._LOG.error(f"{exc}") - if hasattr(self, "location_file"): - self._exit_with_error(ReturnCode.CANNOT_OPEN_LOCATION_FILE) - elif hasattr(self, "product_locator"): - self._exit_with_error(ReturnCode.PRODUCT_LOCATOR_NOT_FOUND) - else: - # should never happen; we've already checked - self._exit_with_error(ReturnCode.MISSING_SETTING) + capo_settings = get_capo_settings(self.profile) + return LocationsReport(self._LOG, self.args, capo_settings) def main(): @@ -161,15 +120,12 @@ def main(): parser = get_arg_parser() try: args = parser.parse_args() - except Exception as exc: - logging.error(f"{exc}") - return exc.value - except SystemExit as exc: - logging.error(f"{exc}") - return exc.value.code - settings = get_capo_settings(args.profile) - datafetcher = DataFetcher(args, settings) - return datafetcher.run() + settings = get_capo_settings(args.profile) + datafetcher = DataFetcher(args, settings) + return datafetcher.run() + except DataFetcherException as exc: + logging.exception(exc.message) + sys.exit(exc.code) if __name__ == "__main__": diff --git a/apps/cli/executables/datafetcher/datafetcher/errors.py b/apps/cli/executables/datafetcher/datafetcher/errors.py index 9c126d569..a8d35475a 100644 --- a/apps/cli/executables/datafetcher/datafetcher/errors.py +++ b/apps/cli/executables/datafetcher/datafetcher/errors.py @@ -1,152 +1,74 @@ """ Custom error definitions for data-fetcher """ import logging -import sys -import traceback -from enum import Enum _LOG = logging.getLogger(__name__) # pylint: disable=W1202 -class Errors(Enum): - """ - Assorted constants and functions involving errors - - """ - - NO_PROFILE = 1 - MISSING_SETTING = 2 - LOCATION_SERVICE_TIMEOUT = 3 - LOCATION_SERVICE_REDIRECTS = 4 - LOCATION_SERVICE_ERROR = 5 - NO_LOCATOR = 6 - FILE_ERROR = 7 - NGAS_SERVICE_TIMEOUT = 8 - NGAS_SERVICE_REDIRECTS = 9 - NGAS_SERVICE_ERROR = 10 - SIZE_MISMATCH = 11 - FILE_EXISTS_ERROR = 12 - FILE_NOT_FOUND_ERROR = 13 - - -class NoProfileException(Exception): - """ throw this if Capo profile can't be determined""" - - -class MissingSettingsException(Exception): - """ throw this if a required setting isn't found in command-line args """ - - -class LocationServiceTimeoutException(Exception): - """ throw this if the location service takes too long to return """ - - -class LocationServiceRedirectsException(Exception): - """throw this if the location service - complains about too many redirects - """ - - -class LocationServiceErrorException(Exception): - """ throw this if the location service falls over """ - - -class NGASServiceTimeoutException(Exception): - """ throw this if the NGAS service time out """ - - -class NGASServiceRedirectsException(Exception): - """ throw this if the NGAS service complains about too many redirects """ - - -class NGASServiceErrorException(Exception): - """ throw this if the NGAS service falls over """ - - -class NoLocatorException(Exception): - """throw this if no product locator could be determined from - command-line arguments - - """ - - -class FileErrorException(Exception): - """throw this if file retriever can't access or create the output - directory - - """ - - -class SizeMismatchException(Exception): - """throw this if the size of the file retrieved - doesn't match expected size - - """ - - -TERMINAL_ERRORS = { - Errors.NO_PROFILE: "no CAPO profile provided", - Errors.MISSING_SETTING: "missing required setting", - Errors.LOCATION_SERVICE_TIMEOUT: "request to locator service timed out", - Errors.LOCATION_SERVICE_REDIRECTS: "too many redirects on locator service", - Errors.LOCATION_SERVICE_ERROR: "catastrophic error on locator service", - Errors.NO_LOCATOR: "product locator not found", - Errors.FILE_ERROR: "not able to open specified location file", - Errors.FILE_EXISTS_ERROR: "specified location file exists", - Errors.NGAS_SERVICE_TIMEOUT: "request to NGAS timed out", - Errors.NGAS_SERVICE_REDIRECTS: "too many redirects on NGAS service", - Errors.NGAS_SERVICE_ERROR: "catastrophic error on NGAS service", - Errors.SIZE_MISMATCH: "retrieved file not expected size", - Errors.FILE_NOT_FOUND_ERROR: "target directory or file not found", -} - - -def get_error_descriptions(): - """ user-friendly reporting of errors """ - - result = "Return Codes:\n" - for error in Errors: - result += "\t{}: {}\n".format(error.value, TERMINAL_ERRORS[error]) +errors = [ + (1, "NoProfileException", "no CAPO profile provided"), + (2, "MissingSettingsException", "missing required setting"), + ( + 3, + "LocationServiceTimeoutException", + "request to locator service timed out", + ), + ( + 4, + "LocationServiceRedirectsException", + "too many redirects on locator service", + ), + ( + 5, + "LocationServiceErrorException", + "catastrophic error on locator service", + ), + (6, "NoLocatorException", "product locator not found"), + (7, "FileErrorException", "not able to open specified location file"), + (8, "NGASServiceTimeoutException", "request to NGAS timed out"), + ( + 9, + "NGASServiceRedirectsException", + "too many redirects on NGAS service", + ), + (10, "NGASServiceErrorException", "catastrophic error on NGAS service"), + (11, "SizeMismatchException", "retrieved file not expected size"), + (12, "FileExistsError", "specified location file exists"), + (13, "FileNotFoundError", "target directory or file not found"), + (14, "NGASFetchError", "trouble retrieving files from NGAS"), +] + + +class DataFetcherException(Exception): + code: int + message: str + + +def define_datafetcher_exception(code, name, message): + result = type(name, (DataFetcherException,), {}) + result.code, result.message = code, message return result -def terminal_error(errno): - """report error, then throw in the towel""" - if errno in TERMINAL_ERRORS: - _LOG.error(TERMINAL_ERRORS[errno]) - else: - _LOG.error("unspecified error {}".format(errno)) - - sys.exit(errno.value) - - -def exception_to_error(exception): - """translate an exception to one of our custom errors""" - switcher = { - "NoProfileException": Errors.NO_PROFILE, - "MissingSettingsException": Errors.MISSING_SETTING, - "LocationServiceTimeoutException": Errors.LOCATION_SERVICE_TIMEOUT, - "LocationServiceRedirectsException": Errors.LOCATION_SERVICE_REDIRECTS, - "LocationServiceErrorException": Errors.LOCATION_SERVICE_ERROR, - "NoLocatorException": Errors.NO_LOCATOR, - "FileErrorException": Errors.FILE_ERROR, - "FileExistsError": Errors.FILE_EXISTS_ERROR, - "NGASServiceTimeoutException": Errors.NGAS_SERVICE_TIMEOUT, - "NGASServiceRedirectsException": Errors.NGAS_SERVICE_REDIRECTS, - "NGASServiceErrorException": Errors.NGAS_SERVICE_ERROR, - "SizeMismatchException": Errors.SIZE_MISMATCH, - "FileNotFoundError": Errors.FILE_NOT_FOUND_ERROR, - } - return switcher.get(exception.__class__.__name__) - - -def terminal_exception(exception): - """Report this exception, then throw in the towel. - should be used by DataFetcher -ONLY- - """ - errorno = exception_to_error(exception) - _LOG.debug(traceback.format_exc()) - _LOG.error(str(exception)) - sys.exit(errorno.value) +# Define the exceptions +NoProfileException = define_datafetcher_exception(*errors[0]) +MissingSettingsException = define_datafetcher_exception(*errors[1]) +LocationServiceTimeoutException = define_datafetcher_exception(*errors[2]) +LocationServiceRedirectsException = define_datafetcher_exception(*errors[3]) +LocationServiceErrorException = define_datafetcher_exception(*errors[4]) +NoLocatorException = define_datafetcher_exception(*errors[5]) +FileErrorException = define_datafetcher_exception(*errors[6]) +NGASServiceTimeoutException = define_datafetcher_exception(*errors[7]) +NGASServiceRedirectsException = define_datafetcher_exception(*errors[8]) +NGASServiceErrorException = define_datafetcher_exception(*errors[9]) +SizeMismatchException = define_datafetcher_exception(*errors[10]) +FileExistsError = define_datafetcher_exception(*errors[11]) +FileNotFoundError = define_datafetcher_exception(*errors[12]) +NGASFetchError = define_datafetcher_exception(*errors[13]) + + +TERMINAL_ERROR_CODES = "Return Codes:\n" +for error_code, exception_name, message in errors: + TERMINAL_ERROR_CODES += f"\t{error_code}: {message}\n" diff --git a/apps/cli/executables/datafetcher/datafetcher/locations_report.py b/apps/cli/executables/datafetcher/datafetcher/locations_report.py index b2655f783..2d11de2ac 100644 --- a/apps/cli/executables/datafetcher/datafetcher/locations_report.py +++ b/apps/cli/executables/datafetcher/datafetcher/locations_report.py @@ -14,6 +14,7 @@ from typing import Dict import requests +from . import errors from .errors import ( LocationServiceTimeoutException, LocationServiceRedirectsException, @@ -146,8 +147,7 @@ class LocationsReport: result = json.load(to_read) return result except FileNotFoundError as err: - self._LOG.error(f"{err}") - raise + raise errors.FileNotFoundError() from err def _get_location_report_from_service(self): """Use 'requests' to fetch the location report from the locator service. @@ -172,8 +172,6 @@ class LocationsReport: raise LocationServiceRedirectsException() from exc_re except requests.exceptions.RequestException as ex: raise LocationServiceErrorException(ex) from ex - except Exception as exc: - self._LOG.error(f"{exc}") if response.status_code == http.HTTPStatus.OK: return response.json() diff --git a/apps/cli/executables/datafetcher/datafetcher/project_fetcher.py b/apps/cli/executables/datafetcher/datafetcher/project_fetcher.py index a104a2651..fddf598c9 100644 --- a/apps/cli/executables/datafetcher/datafetcher/project_fetcher.py +++ b/apps/cli/executables/datafetcher/datafetcher/project_fetcher.py @@ -7,11 +7,9 @@ import sys from argparse import Namespace from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path -from pprint import pprint from typing import Dict -from datafetcher.errors import NGASServiceErrorException -from datafetcher.return_codes import ReturnCode +from datafetcher.errors import NGASServiceErrorException, NGASFetchError from .file_retrievers import NGASFileRetriever from .utilities import FlexLogger @@ -133,71 +131,30 @@ class ParallelFetcher(BaseFetcher): def fetch_bucket(self, bucket): """ Grab the files in this bucket """ - file_sizes = [file["size"] for file in bucket["files"]] - pprint(f"retrieving files {file_sizes} from {bucket['server']}") self._LOG.debug( f"{bucket['retrieve_method']} " f"{len(bucket['files'])} files from " f"{bucket['server']}...." ) - num_files = self.retrieve_files( - bucket["server"], bucket["retrieve_method"], bucket["files"] - ) - print(f"done retrieving files {file_sizes} from {bucket['server']}") - return num_files + self.retrieve_files(bucket["server"], bucket["retrieve_method"], bucket["files"]) def run(self): """ Fetch all the files for the product locator """ - print(f"running {self.__class__.__name__}") if self.args.dry_run: self._LOG.debug("This is a dry run; files will not be fetched") return 0 with ThreadPoolExecutor() as executor: results = executor.map(self.fetch_bucket, self.bucketized_files) - print(f"results: {results}") - try: - for result in results: - print(f"result has arrived: {result}") - self.num_files_retrieved += result - if self.num_files_retrieved != self.num_files_expected: - self._LOG.error( - f"{self.num_files_expected} files expected, " - f"but only {self.num_files_retrieved} retrieved" - ) - self._exit_with_error(ReturnCode.NGAS_FETCH_ERROR) - - # successful retrieval - print("returning") - return 0 - except (FileExistsError, NGASServiceErrorException) as exc: - self._LOG.error(f"{exc}") - self._exit_with_error(ReturnCode.NGAS_FETCH_ERROR) - except AttributeError: - # (This error sometimes gets thrown after all files - # actually -have- been retrieved. I blame the NGAS API.) - - output_path = Path(self.args.output_dir) - files = [ - file - for file in output_path.rglob("*") - if not file.is_dir() - and not str(file).endswith(".json") - and not str(file).endswith(".log") - ] - if len(files) < self.num_files_expected: - self._LOG.error( - f"{self.num_files_expected} files expected, but only " - f"{self.num_files_retrieved} found" - ) - self._exit_with_error(ReturnCode.CATASTROPHIC_REQUEST_ERROR) - - return 0 - - except Exception as exc: - self._LOG.error(f"{exc}") - self._exit_with_error(ReturnCode.NGAS_FETCH_ERROR) - - def _exit_with_error(self, return_code: ReturnCode): - sys.exit(return_code.value["code"]) + futures = as_completed(results) + for future in futures: + self.num_files_retrieved += future.result() + if self.num_files_retrieved != self.num_files_expected: + raise NGASFetchError( + f"{self.num_files_expected} files expected, " + f"but only {self.num_files_retrieved} retrieved" + ) + + # successful retrieval + return 0 diff --git a/apps/cli/executables/datafetcher/datafetcher/return_codes.py b/apps/cli/executables/datafetcher/datafetcher/return_codes.py index cba3ac2e8..e69de29bb 100644 --- a/apps/cli/executables/datafetcher/datafetcher/return_codes.py +++ b/apps/cli/executables/datafetcher/datafetcher/return_codes.py @@ -1,17 +0,0 @@ -""" data-fetcher return codes as specified in README and usage string """ -from enum import Enum - - -class ReturnCode(Enum): - """ Canonical data-fetcher exit code values """ - - SUCCESS = {"code": 0, "text": "success"} - MISSING_PROFILE = {"code": 1, "text": "no CAPO profile provided"} - MISSING_SETTING = {"code": 2, "text": "missing required setting"} - LOCATOR_SERVICE_TIMEOUT = {"code": 3, "text": "request to locator service timed out"} - TOO_MANY_SERVICE_REDIRECTS = {"code": 4, "text": "too many redirects on locator service"} - CATASTROPHIC_REQUEST_ERROR = {"code": 5, "text": "catastrophic error on request service"} - PRODUCT_LOCATOR_NOT_FOUND = {"code": 6, "text": "product locator not found"} - CANNOT_OPEN_LOCATION_FILE = {"code": 7, "text": "not able to open specified location file"} - NGAS_FETCH_ERROR = {"code": 8, "text": "error fetching file from NGAS server"} - SIZE_MISMATCH = {"code": 9, "text": "retrieved file not expected size"} diff --git a/apps/cli/executables/datafetcher/datafetcher/utilities.py b/apps/cli/executables/datafetcher/datafetcher/utilities.py index ee0e1d3d3..0d21058b0 100644 --- a/apps/cli/executables/datafetcher/datafetcher/utilities.py +++ b/apps/cli/executables/datafetcher/datafetcher/utilities.py @@ -8,22 +8,22 @@ """ import argparse +from enum import Enum import logging import os import pathlib -from enum import Enum -from time import time +import time from typing import Callable import psycopg2 as pg from pycapo import CapoConfig from .errors import ( - get_error_descriptions, NoProfileException, MissingSettingsException, NGASServiceErrorException, SizeMismatchException, + TERMINAL_ERROR_CODES, ) @@ -50,7 +50,7 @@ SERVER_SPEC_KEYS = ["server", "location", "cluster", "retrieve_method"] _PROLOGUE = """Retrieve a product (a science product or an ancillary product) from the NRAO archive, either by specifying the product's locator or by providing the path to a product locator report.""" -_EPILOGUE = get_error_descriptions() +_EPILOGUE = TERMINAL_ERROR_CODES # This is a dictionary of required CAPO settings # and the attribute names we'll store them as. @@ -167,12 +167,11 @@ def get_capo_settings(profile: str): for setting in REQUIRED_SETTINGS: setting = setting.upper() try: - value = capo[setting] + result[REQUIRED_SETTINGS[setting]] = capo[setting] except KeyError as k_err: raise MissingSettingsException( 'missing required setting "{}"'.format(setting) ) from k_err - result[REQUIRED_SETTINGS[setting]] = value return result @@ -226,9 +225,7 @@ class ProductLocatorLookup: password=self.credentials["jdbcPassword"], ) as conn: cursor = conn.cursor() - sql = ( - "SELECT science_product_locator " "FROM science_products " "WHERE external_name=%s" - ) + sql = "SELECT science_product_locator FROM science_products WHERE external_name=%s" cursor.execute( sql, (external_name,), @@ -254,7 +251,7 @@ class FlexLogger: if class_name is None: raise MissingSettingsException("class name is required") - log_pathname = f"{output_dir}/{class_name}_{str(time())}.log" + log_pathname = f"{output_dir}/{class_name}_{str(time.time())}.log" try: self.logfile = pathlib.Path(output_dir, log_pathname) @@ -320,9 +317,6 @@ class Retryer: :param args: :return: """ - - import time - while self.num_tries < self.max_tries and not self.complete: self.num_tries += 1 diff --git a/apps/cli/executables/datafetcher/test/df_pytest_utils.py b/apps/cli/executables/datafetcher/test/df_pytest_utils.py index 9ea070526..060e06526 100644 --- a/apps/cli/executables/datafetcher/test/df_pytest_utils.py +++ b/apps/cli/executables/datafetcher/test/df_pytest_utils.py @@ -42,7 +42,6 @@ from shared.workspaces.test.test_data.utilities import ( ) from datafetcher.datafetcher import DataFetcher -from datafetcher.return_codes import ReturnCode from datafetcher.errors import MissingSettingsException, NoProfileException from datafetcher.locations_report import LocationsReport from datafetcher.utilities import ( @@ -54,8 +53,6 @@ from datafetcher.utilities import ( ) TEST_PROFILE = "docker" -MISSING_SETTING = ReturnCode.MISSING_SETTING.value["code"] -MISSING_PROFILE = ReturnCode.MISSING_PROFILE.value["code"] RUN_ALL = True LOCATION_REPORTS = { @@ -98,19 +95,6 @@ LOCATION_REPORTS = { } -def get_locations_file(key: str): - """ - Return location report file specified by key - :param key: location report name - :return: - - """ - - report_spec = LOCATION_REPORTS[key] - filename = report_spec["filename"] - return Path(get_test_data_dir(), filename) - - def write_locations_file(destination: Path, locations_report: LocationsReport): """ @@ -322,7 +306,7 @@ def launch_datafetcher(args: list, df_capo_settings: dict) -> int: """ if args is None or len(args) == 0: - return MISSING_SETTING + return MissingSettingsException.code try: namespace = evaluate_args_and_capo(args, df_capo_settings) @@ -337,7 +321,7 @@ def launch_datafetcher(args: list, df_capo_settings: dict) -> int: raise except (KeyError, NoProfileException) as exc: logging.error(f"{exc}") - return MISSING_PROFILE + return NoProfileException.code except Exception as exc: pytest.fail(f"{exc}") @@ -345,13 +329,13 @@ def launch_datafetcher(args: list, df_capo_settings: dict) -> int: def evaluate_args_and_capo(args: list, capo_settings: dict): if args is None or len(args) == 0: - sys.exit(MISSING_SETTING) + sys.exit(MissingSettingsException.code) profile = get_profile_from_args(args) if profile is None: profile = capo_settings["profile"] if profile is None: - sys.exit(MISSING_PROFILE) + sys.exit(NoProfileException.code) else: args["profile"] = profile diff --git a/apps/cli/executables/datafetcher/test/mock_data_fetcher.py b/apps/cli/executables/datafetcher/test/mock_data_fetcher.py index cbadf65e5..8c718b2de 100644 --- a/apps/cli/executables/datafetcher/test/mock_data_fetcher.py +++ b/apps/cli/executables/datafetcher/test/mock_data_fetcher.py @@ -5,8 +5,8 @@ from argparse import Namespace from datafetcher.locations_report import LocationsReport from datafetcher.project_fetcher import ParallelFetcher -from datafetcher.return_codes import ReturnCode from datafetcher.utilities import get_capo_settings, ExecutionSite, FlexLogger +from datafetcher.errors import MissingSettingsException from .df_pytest_utils import TEST_PROFILE @@ -18,7 +18,7 @@ class MockProdDataFetcher: def __init__(self, args: Namespace, settings: dict): if args is None or settings is None: - self._exit_with_error(ReturnCode.MISSING_SETTING) + raise MissingSettingsException() self.args = args self.settings = settings self.verbose = self.args.verbose @@ -28,19 +28,8 @@ class MockProdDataFetcher: self._LOG = FlexLogger(self.__class__.__name__, self.output_dir, self.verbose) - try: - self.locations_report = self._get_locations() - self.servers_report = self.locations_report.servers_report - except SystemExit: - if args.location_file: - self._exit_with_error(ReturnCode.CANNOT_OPEN_LOCATION_FILE) - else: - self._exit_with_error(ReturnCode.PRODUCT_LOCATOR_NOT_FOUND) - raise - - except Exception as exc: - self._LOG.error(f">>> throwing unexpected {type(exc)} during init: {exc}") - raise + self.locations_report = self._get_locations() + self.servers_report = self.locations_report.servers_report def _get_locations(self): """ @@ -60,14 +49,4 @@ class MockProdDataFetcher: :return: """ - try: - return ParallelFetcher(self.args, self.settings, self._LOG, self.servers_report).run() - except SystemExit as exc: - self._LOG.error(f"{exc}") - raise - except Exception as exc: - self._LOG.error(f">>> throwing unexpected exception during run: {exc}") - raise - - def _exit_with_error(self, return_code: ReturnCode): - sys.exit(return_code.value["code"]) + return ParallelFetcher(self.args, self.settings, self._LOG, self.servers_report).run() diff --git a/apps/cli/executables/datafetcher/test/test_df_function.py b/apps/cli/executables/datafetcher/test/test_df_function.py index 5df3d7a45..be23c3e41 100644 --- a/apps/cli/executables/datafetcher/test/test_df_function.py +++ b/apps/cli/executables/datafetcher/test/test_df_function.py @@ -5,6 +5,8 @@ from pathlib import Path import pytest import sys +from datafetcher.errors import NGASFetchError, MissingSettingsException + sys.path.insert(0, str(Path(".").absolute())) from .df_pytest_utils import get_project_root @@ -14,7 +16,7 @@ sys.path.insert(0, str(project_root)) # pylint: disable=C0115, C0116, C0200, C0415, R0801, R0902, R0903, R0914, R1721, W0212, W0611, W0613, W0621, W0703, W1203 -from datafetcher.datafetcher import DataFetcher, ReturnCode +from datafetcher.datafetcher import DataFetcher from datafetcher.utilities import ( get_arg_parser, ProductLocatorLookup, @@ -57,16 +59,6 @@ def test_settings_setup(settings): assert isinstance(settings.test_data, dict) -@pytest.mark.skipif(not RUN_ALL, reason="debug") -def test_usage_statement_makes_sense(): - """ Ensure that the datafetcher's "usage" statement is as we expect """ - - usage = DataFetcher._build_usage_message() - assert usage.startswith("Usage:") - lines = usage.split("\n") - assert len(lines) >= len(ReturnCode) + 1 - - @pytest.mark.skipif(not RUN_ALL, reason="debug") def test_nothing_retrieved_if_dry_locator(make_tempdir, settings): """ Simulates dry run with product locator """ @@ -173,7 +165,7 @@ def test_no_overwrite_without_force(make_tempdir, settings): ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.NGAS_FETCH_ERROR.value["code"] + assert return_code == NGASFetchError.code sizes = dict() for file in dest_dir.rglob("*"): @@ -199,7 +191,7 @@ def test_more_output_when_verbose(make_tempdir, settings): ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.SUCCESS.value["code"] + assert return_code == 0 num_files_expected = 37 retrieved = [file for file in top_level.rglob("*") if file.is_file()] @@ -230,7 +222,7 @@ def test_more_output_when_verbose(make_tempdir, settings): ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.SUCCESS.value["code"] + assert return_code == 0 retrieved = [file for file in top_level.rglob("*")] assert len(retrieved) == num_files_expected + 3 @@ -296,7 +288,7 @@ def test_copy_attempt_throws_sys_exit_service_error(make_tempdir, settings): with pytest.raises(SystemExit) as exc: fetcher.run() - assert exc.value.code == ReturnCode.CATASTROPHIC_REQUEST_ERROR.value["code"] + assert exc.value.code == NGASFetchError.code finally: if props_file.exists(): props_file.unlink() @@ -317,7 +309,7 @@ def test_dies_with_bad_server_info(make_tempdir, settings): try: launch_datafetcher(args, settings.capo_settings) except Exception as exc: - assert exc.value.code == ReturnCode.NGAS_FETCH_ERROR["code"] + assert exc.value.code == NGASFetchError.code @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -333,7 +325,7 @@ def test_missing_setting_error_on_bad_destination(settings): try: launch_datafetcher(args, settings.capo_settings) except Exception as exc: - assert exc.value.code == ReturnCode.MISSING_SETTING["code"] + assert exc.value.code == MissingSettingsException.code def write_fake_file(destination: Path, file_info: dict): diff --git a/apps/cli/executables/datafetcher/test/test_df_return_codes.py b/apps/cli/executables/datafetcher/test/test_df_return_codes.py index 36c6c512c..177fd9501 100644 --- a/apps/cli/executables/datafetcher/test/test_df_return_codes.py +++ b/apps/cli/executables/datafetcher/test/test_df_return_codes.py @@ -9,8 +9,16 @@ from pathlib import Path import pytest +from datafetcher.errors import ( + NoProfileException, + MissingSettingsException, + LocationServiceTimeoutException, + NGASServiceRedirectsException, + NGASFetchError, + NoLocatorException, + SizeMismatchException, +) from datafetcher.datafetcher import DataFetcher -from datafetcher.return_codes import ReturnCode from datafetcher.utilities import get_arg_parser, ProductLocatorLookup # N.B. IJ doesn't recognize imported fixtures as being in use. @@ -22,8 +30,6 @@ from .df_pytest_utils import ( capo_settings, settings, launch_datafetcher, - MISSING_SETTING, - MISSING_PROFILE, RUN_ALL, confirm_retrieve_mode_copy, ) @@ -94,7 +100,7 @@ def test_omitted_profile_returns_expected_code(make_tempdir, settings): ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == MISSING_PROFILE + assert return_code == NoProfileException.code # restore the existing CAPO_PROFILE os.environ["CAPO_PROFILE"] = existing_capo_profile @@ -119,7 +125,7 @@ def test_omitted_capo_value_returns_expected_code(make_tempdir, settings): TEST_PROFILE, ] result = launch_datafetcher(args, settings.capo_settings) - assert result == MISSING_SETTING + assert result == MissingSettingsException.code @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -146,7 +152,7 @@ def test_invalid_capo_profile_returns_expected_code(make_tempdir, settings): str(make_tempdir), ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == MISSING_PROFILE + assert return_code == NoProfileException.code def we_are_in_docker(): @@ -231,7 +237,7 @@ def test_two_locator_args_returns_expected_code(make_tempdir, settings): TEST_PROFILE, ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == MISSING_SETTING + assert return_code == MissingSettingsException.code class MockServiceTimeoutReturn: @@ -239,7 +245,7 @@ class MockServiceTimeoutReturn: @staticmethod def run(): - return ReturnCode.LOCATOR_SERVICE_TIMEOUT.value["code"] + raise LocationServiceTimeoutException() @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -257,13 +263,13 @@ def test_locator_service_timeout_returns_expected_code(monkeypatch, settings, ma ] monkeypatch.setattr(DataFetcher, "run", mock_run) return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.LOCATOR_SERVICE_TIMEOUT.value["code"] + assert return_code == LocationServiceTimeoutException.code class MockTooManyServiceRedirectsReturn: @staticmethod def run(): - return ReturnCode.TOO_MANY_SERVICE_REDIRECTS.value + raise NGASServiceRedirectsException() @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -281,13 +287,13 @@ def test_too_many_service_redirects_returns_expected_code(monkeypatch, settings, ] monkeypatch.setattr(DataFetcher, "run", mock_run) return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.TOO_MANY_SERVICE_REDIRECTS.value + assert return_code == NGASServiceRedirectsException.code class MockCatastrophicServiceErrorReturn: @staticmethod def run(): - return ReturnCode.CATASTROPHIC_REQUEST_ERROR + raise NGASFetchError() @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -306,7 +312,7 @@ def test_catastrophic_service_error_returns_expected_code(monkeypatch, settings, monkeypatch.setattr(DataFetcher, "run", mock_run) return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.CATASTROPHIC_REQUEST_ERROR + assert return_code == NGASFetchError.code def test_copy_attempt_throws_sys_exit_service_error(monkeypatch, settings, make_tempdir): @@ -339,7 +345,7 @@ def test_copy_attempt_throws_sys_exit_service_error(monkeypatch, settings, make_ with pytest.raises(SystemExit) as exc: fetcher.run() - assert exc.value.code == ReturnCode.CATASTROPHIC_REQUEST_ERROR.value["code"] + assert exc.value.code == NGASFetchError.code @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -353,7 +359,7 @@ def test_product_locator_not_found_returns_expected_code(make_tempdir, settings) TEST_PROFILE, ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.PRODUCT_LOCATOR_NOT_FOUND.value["code"] + assert return_code == NoLocatorException.code @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -367,7 +373,7 @@ def test_unable_to_open_location_file_returns_expected_code(make_tempdir, settin TEST_PROFILE, ] return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.CANNOT_OPEN_LOCATION_FILE.value["code"] + assert return_code == LocationServiceTimeoutException.code class MockNgasFetchError: @@ -375,7 +381,7 @@ class MockNgasFetchError: @staticmethod def run(): - return ReturnCode.NGAS_FETCH_ERROR + raise NGASFetchError() @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -393,7 +399,7 @@ def test_error_fetching_file_from_ngas_returns_expected_code(monkeypatch, settin ] monkeypatch.setattr(DataFetcher, "run", mock_run) return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.NGAS_FETCH_ERROR + assert return_code == NGASFetchError.code class MockSizeMismatchError: @@ -401,7 +407,7 @@ class MockSizeMismatchError: @staticmethod def run(): - return ReturnCode.SIZE_MISMATCH + raise SizeMismatchException() @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -419,4 +425,4 @@ def test_unexpected_size_returns_expected_code(monkeypatch, settings, make_tempd ] monkeypatch.setattr(DataFetcher, "run", mock_run) return_code = launch_datafetcher(args, settings.capo_settings) - assert return_code == ReturnCode.SIZE_MISMATCH + assert return_code == SizeMismatchException.code -- GitLab