diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3501a2a..0df099c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -4,7 +4,7 @@ stages: test: stage: test - image: python:3.8.7-slim-buster + image: python:3.8.7-slim-bullseye before_script: - apt-get update && apt-get install curl default-jre -y - mkdir /tmp/nextflow && cd /tmp/nextflow && (curl -s https://get.nextflow.io | bash) && chmod 755 nextflow $$ export PATH=/tmp/:$PATH && cd - @@ -29,7 +29,7 @@ test: deploy-python-package-production: stage: deploy - image: python:3.8.7-slim-buster + image: python:3.8.7-slim-bullseye script: - python setup.py sdist - twine upload dist/* @@ -41,4 +41,3 @@ deploy-python-package-production: TWINE_PASSWORD: $PYPI_PASSWORD only: - tags - diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e042b..1bea117 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ Changelog for ebi_eva_common_pyutils ## 0.7.4 (unreleased) --------------------- -- Nothing changed yet. +- Update biosamples communicator to not retry 404s ## 0.7.3 (2025-06-19) diff --git a/ebi_eva_common_pyutils/biosamples_communicators.py b/ebi_eva_common_pyutils/biosamples_communicators.py index 5b3df92..e378509 100644 --- a/ebi_eva_common_pyutils/biosamples_communicators.py +++ b/ebi_eva_common_pyutils/biosamples_communicators.py @@ -25,11 +25,18 @@ class HALNotReadyError(Exception): pass +# Extends ValueError so clients can still expect ValueErrors to be raised on failed requests, +# but we have a more specific class internally to know when to retry. +class HttpErrorRetry(ValueError): + pass + + class HALCommunicator(AppLogger): """ This class helps navigate through REST API that uses the HAL standard. """ acceptable_code = [200, 201] + no_retry_code = [404] def __init__(self, auth_url, bsd_url, username, password): self.auth_url = auth_url @@ -43,9 +50,12 @@ def _validate_response(self, response): self.error(response.request.method + ': ' + response.request.url + " with " + str(response.request.body)) self.error("headers: {}".format(response.request.headers)) self.error("<{}>: {}".format(response.status_code, response.text)) - raise ValueError('The HTTP status code ({}) is not one of the acceptable codes ({})'.format( + error_msg = 'The HTTP status code ({}) is not one of the acceptable codes ({})'.format( str(response.status_code), str(self.acceptable_code)) - ) + if response.status_code in self.no_retry_code: + raise ValueError(error_msg) + else: + raise HttpErrorRetry(error_msg) return response @cached_property @@ -55,7 +65,7 @@ def token(self): self._validate_response(response) return response.text - @retry(exceptions=(ValueError, requests.RequestException), tries=3, delay=2, backoff=1.2, jitter=(1, 3)) + @retry(exceptions=(HttpErrorRetry, requests.RequestException), tries=3, delay=2, backoff=1.2, jitter=(1, 3)) def _req(self, method, url, **kwargs): """Private method that sends a request using the specified method. It adds the headers required by bsd""" headers = kwargs.pop('headers', {}) diff --git a/setup.py b/setup.py index be71eaf..11d3ef4 100644 --- a/setup.py +++ b/setup.py @@ -17,7 +17,7 @@ url='https://github.com/EBIVariation/eva-common-pyutils', keywords=['EBI', 'EVA', 'PYTHON', 'UTILITIES'], install_requires=requirements, - extras_require={'eva-internal': ['psycopg2-binary', 'pymongo', 'networkx<=2.5']}, + extras_require={'eva-internal': ['psycopg2-binary', 'pymongo<=3.12', 'networkx<=2.5']}, classifiers=[ 'Development Status :: 5 - Production/Stable', 'Intended Audience :: Developers', diff --git a/tests/common/test_biosamples_communicators.py b/tests/common/test_biosamples_communicators.py index 367844e..9d0cf6a 100644 --- a/tests/common/test_biosamples_communicators.py +++ b/tests/common/test_biosamples_communicators.py @@ -2,7 +2,7 @@ from unittest import TestCase from unittest.mock import Mock, patch, PropertyMock -from ebi_eva_common_pyutils.biosamples_communicators import HALCommunicator, WebinHALCommunicator +from ebi_eva_common_pyutils.biosamples_communicators import HALCommunicator, WebinHALCommunicator, HttpErrorRetry class TestHALCommunicator(TestCase): @@ -29,10 +29,22 @@ def test_req(self): headers={'Accept': 'application/hal+json', 'Authorization': 'Bearer token'} ) + # 500 should trigger a retry with patch.object(HALCommunicator, 'token', new_callable=PropertyMock(return_value='token')), \ patch('requests.request') as mocked_request: mocked_request.return_value = Mock(status_code=500, request=PropertyMock(url='text')) - self.assertRaises(ValueError, self.comm._req, 'GET', 'http://BSD.example.org') + self.assertRaises(HttpErrorRetry, self.comm._req, 'GET', 'http://BSD.example.org') + + # 404 should fail with a ValueError, but not trigger a retry + with patch.object(HALCommunicator, 'token', new_callable=PropertyMock(return_value='token')), \ + patch('requests.request') as mocked_request: + mocked_request.return_value = Mock(status_code=404, request=PropertyMock(url='text')) + try: + self.comm._req('GET', 'http://BSD.example.org') + except HttpErrorRetry: + self.fail('Request should not raise HttpErrorRetry') + except ValueError: + return def test_root(self): expected_json = {'json': 'values'}