diff --git a/CHANGELOG.md b/CHANGELOG.md index 156d06b..037358d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## v1.0.1 +### 2023-12-19 + +This version of HOSS removes custom, redundant download retry logic. Instead +retries are relied upon from `harmony-service-lib` and for each stage in a +Harmony workflow. + ## v1.0.0 ### 2023-10-06 diff --git a/docker/service_version.txt b/docker/service_version.txt index 3eefcb9..7dea76e 100644 --- a/docker/service_version.txt +++ b/docker/service_version.txt @@ -1 +1 @@ -1.0.0 +1.0.1 diff --git a/hoss/bbox_utilities.py b/hoss/bbox_utilities.py index d87af68..e762082 100644 --- a/hoss/bbox_utilities.py +++ b/hoss/bbox_utilities.py @@ -58,6 +58,7 @@ def get_request_shape_file(message: Message, working_dir: str, raise UnsupportedShapeFileFormat(message.subset.shape.type) shape_file_url = message.subset.shape.process('href') + adapter_logger.info('Downloading request shape file') local_shape_file_path = download(shape_file_url, working_dir, logger=adapter_logger, access_token=message.accessToken, diff --git a/hoss/exceptions.py b/hoss/exceptions.py index 216c7f5..a3a05eb 100644 --- a/hoss/exceptions.py +++ b/hoss/exceptions.py @@ -119,14 +119,3 @@ class UrlAccessFailed(CustomError): def __init__(self, url, status_code): super().__init__('UrlAccessFailed', f'{status_code} error retrieving: {url}') - - -class UrlAccessFailedWithRetries(CustomError): - """ This exception is raised when an HTTP request for a given URL has - failed a specified number of times. - - """ - def __init__(self, url): - super().__init__('UrlAccessFailedWithRetries', - f'URL: {url} was unsuccessfully requested the ' - 'maximum number of times.') diff --git a/hoss/utilities.py b/hoss/utilities.py index 9da61ca..4f42e98 100644 --- a/hoss/utilities.py +++ b/hoss/utilities.py @@ -15,10 +15,7 @@ from harmony.exceptions import ForbiddenException, ServerException from harmony.util import Config, download as util_download -from hoss.exceptions import UrlAccessFailed, UrlAccessFailedWithRetries - - -HTTP_REQUEST_ATTEMPTS = 3 +from hoss.exceptions import UrlAccessFailed def get_file_mimetype(file_name: str) -> Tuple[Optional[str], Optional[str]]: @@ -90,12 +87,12 @@ def download_url(url: str, destination: str, logger: Logger, access_token: str = None, config: Config = None, data=None) -> str: """ Use built-in Harmony functionality to download from a URL. This is - expected to be used for obtaining the granule `.dmr` and the granule - itself (only the required variables). + expected to be used for obtaining the granule `.dmr`, a prefetch of + only dimensions and bounds variables, and the subsetted granule itself. - OPeNDAP can return intermittent 500 errors. This function will retry - the original request in the event of a 500 error, but not for other - error types. In those instances, the original HTTPError is re-raised. + OPeNDAP can return intermittent 500 errors. Retries will be performed + by inbuilt functionality in the `harmony-service-lib`. The OPeNDAP + errors are captured and re-raised as custom exceptions. The return value is the location in the file-store of the downloaded content from the URL. @@ -106,32 +103,21 @@ def download_url(url: str, destination: str, logger: Logger, if data is not None: logger.info(f'POST request data: "{format_dictionary_string(data)}"') - request_completed = False - attempts = 0 - - while not request_completed and attempts < HTTP_REQUEST_ATTEMPTS: - attempts += 1 - - try: - response = util_download( - url, - destination, - logger, - access_token=access_token, - data=data, - cfg=config - ) - request_completed = True - except ForbiddenException as harmony_exception: - raise UrlAccessFailed(url, 400) from harmony_exception - except ServerException as harmony_exception: - if attempts < HTTP_REQUEST_ATTEMPTS: - logger.info('500 error returned, retrying request.') - else: - raise UrlAccessFailedWithRetries(url) from harmony_exception - except Exception as harmony_exception: - # Not a 500 error, so raise immediately and exit the loop. - raise UrlAccessFailed(url, 'Unknown') from harmony_exception + try: + response = util_download( + url, + destination, + logger, + access_token=access_token, + data=data, + cfg=config + ) + except ForbiddenException as harmony_exception: + raise UrlAccessFailed(url, 400) from harmony_exception + except ServerException as harmony_exception: + raise UrlAccessFailed(url, 500) from harmony_exception + except Exception as harmony_exception: + raise UrlAccessFailed(url, 'Unknown') from harmony_exception return response diff --git a/tests/unit/test_utilities.py b/tests/unit/test_utilities.py index 1698f08..24043d5 100644 --- a/tests/unit/test_utilities.py +++ b/tests/unit/test_utilities.py @@ -5,13 +5,12 @@ from harmony.exceptions import ForbiddenException, ServerException from harmony.util import config -from hoss.exceptions import UrlAccessFailed, UrlAccessFailedWithRetries +from hoss.exceptions import UrlAccessFailed from hoss.utilities import (download_url, format_dictionary_string, format_variable_set_string, get_constraint_expression, get_file_mimetype, get_opendap_nc4, get_value_or_default, - HTTP_REQUEST_ATTEMPTS, move_downloaded_nc4, - rgetattr) + move_downloaded_nc4, rgetattr) class TestUtilities(TestCase): @@ -92,10 +91,18 @@ def test_download_url(self, mock_util_download): mock_util_download.side_effect = [self.harmony_500_error, http_response] - response = download_url(test_url, output_directory, self.logger) + with self.assertRaises(UrlAccessFailed): + download_url(test_url, output_directory, self.logger, + access_token, self.config) - self.assertEqual(response, http_response) - self.assertEqual(mock_util_download.call_count, 2) + mock_util_download.assert_called_once_with( + test_url, + output_directory, + self.logger, + access_token=access_token, + data=None, + cfg=self.config + ) mock_util_download.reset_mock() with self.subTest('Non-500 error does not retry, and is re-raised.'): @@ -116,17 +123,6 @@ def test_download_url(self, mock_util_download): ) mock_util_download.reset_mock() - with self.subTest('Maximum number of attempts not exceeded.'): - mock_util_download.side_effect = [ - self.harmony_500_error - ] * (HTTP_REQUEST_ATTEMPTS + 1) - with self.assertRaises(UrlAccessFailedWithRetries): - download_url(test_url, output_directory, self.logger) - - self.assertEqual(mock_util_download.call_count, - HTTP_REQUEST_ATTEMPTS) - mock_util_download.reset_mock() - @patch('hoss.utilities.move_downloaded_nc4') @patch('hoss.utilities.util_download') def test_get_opendap_nc4(self, mock_download, mock_move_download):