diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index f129278..f29b836 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -35,10 +35,10 @@ jobs: run: docker login --username "$DOCKERHUB_USER" --password "$DOCKERHUB_TOKEN" - name: Build image - run: docker-compose build ci + run: docker compose build ci - name: Check image - run: docker-compose run --rm ci -m flake8 + run: docker compose run --rm ci -m flake8 - name: Set image tag id: tag @@ -69,7 +69,7 @@ jobs: - name: Run Tests run: | - docker-compose run --rm -e KBC_TEST_TOKEN=$KBC_TEST_TOKEN -e KBC_TEST_API_URL=$KBC_TEST_API_URL -e SKIP_ABS_TESTS=1 ci -m unittest --verbose + docker compose run --rm -e KBC_TEST_TOKEN=$KBC_TEST_TOKEN -e KBC_TEST_API_URL=$KBC_TEST_API_URL -e SKIP_ABS_TESTS=1 ci -m unittest --verbose tests_azure: name: Run tests (Azure) @@ -86,7 +86,7 @@ jobs: - name: Run Tests run: | - docker-compose run --rm -e KBC_TEST_TOKEN=$KBC_AZ_TEST_TOKEN -e KBC_TEST_API_URL=$KBC_AZ_TEST_API_URL -e SKIP_ABS_TESTS=1 ci -m unittest --verbose + docker compose run --rm -e KBC_TEST_TOKEN=$KBC_AZ_TEST_TOKEN -e KBC_TEST_API_URL=$KBC_AZ_TEST_API_URL -e SKIP_ABS_TESTS=1 ci -m unittest --verbose tests_gcp: name: Run tests (GCP) @@ -103,7 +103,7 @@ jobs: - name: Run Tests run: | - docker-compose run --rm -e KBC_TEST_TOKEN=$KBC_GCP_TEST_TOKEN -e KBC_TEST_API_URL=$KBC_GCP_TEST_API_URL -e SKIP_ABS_TESTS=1 ci -m unittest --verbose + docker compose run --rm -e KBC_TEST_TOKEN=$KBC_GCP_TEST_TOKEN -e KBC_TEST_API_URL=$KBC_GCP_TEST_API_URL -e SKIP_ABS_TESTS=1 ci -m unittest --verbose deploy_to_pypi: needs: diff --git a/README.md b/README.md index 1421d1c..969c176 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ tables.detail('in.c-demo.some-table') Create `.env` file according to the `.env.template` file and run the tests with: ```bash -$ docker-compose run --rm -e KBC_TEST_TOKEN -e KBC_TEST_API_URL sapi-python-client -m unittest discover +$ docker compose run --rm -e KBC_TEST_TOKEN -e KBC_TEST_API_URL sapi-python-client -m unittest discover ``` ## Contribution Guide diff --git a/kbcstorage/base.py b/kbcstorage/base.py index 5b12f53..a8d481d 100644 --- a/kbcstorage/base.py +++ b/kbcstorage/base.py @@ -9,6 +9,7 @@ .. _Storage API documentation: http://docs.keboola.apiary.io/ """ +from kbcstorage.retry_requests import MAX_RETRIES_DEFAULT, RetryRequests import requests @@ -21,7 +22,7 @@ class Endpoint: base_url (str): The base URL for this endpoint. token (str): A key for the Storage API. """ - def __init__(self, root_url, path_component, token): + def __init__(self, root_url, path_component, token, max_requests_retries=MAX_RETRIES_DEFAULT): """ Create an endpoint. @@ -44,6 +45,7 @@ def __init__(self, root_url, path_component, token): self._auth_header = {'X-StorageApi-Token': self.token, 'Accept-Encoding': 'gzip', 'User-Agent': 'Keboola Storage API Python Client'} + self.requests = RetryRequests(max_requests_retries) def _get_raw(self, url, params=None, **kwargs): """ @@ -66,7 +68,7 @@ def _get_raw(self, url, params=None, **kwargs): headers = kwargs.pop('headers', {}) headers.update(self._auth_header) - r = requests.get(url, params, headers=headers, **kwargs) + r = self.requests.get(url, params=params, headers=headers, **kwargs) try: r.raise_for_status() except requests.HTTPError: @@ -111,7 +113,7 @@ def _post(self, *args, **kwargs): """ headers = kwargs.pop('headers', {}) headers.update(self._auth_header) - r = requests.post(headers=headers, *args, **kwargs) + r = self.requests.post(headers=headers, *args, **kwargs) try: r.raise_for_status() except requests.HTTPError: @@ -137,7 +139,7 @@ def _put(self, *args, **kwargs): """ headers = kwargs.pop('headers', {}) headers.update(self._auth_header) - r = requests.put(headers=headers, *args, **kwargs) + r = self.requests.put(headers=headers, *args, **kwargs) try: r.raise_for_status() except requests.HTTPError: @@ -163,7 +165,7 @@ def _delete(self, *args, **kwargs): """ headers = kwargs.pop('headers', {}) headers.update(self._auth_header) - r = requests.delete(headers=headers, *args, **kwargs) + r = self.requests.delete(headers=headers, *args, **kwargs) try: r.raise_for_status() except requests.HTTPError: diff --git a/kbcstorage/retry_requests.py b/kbcstorage/retry_requests.py new file mode 100644 index 0000000..fba0920 --- /dev/null +++ b/kbcstorage/retry_requests.py @@ -0,0 +1,35 @@ +import time +import requests + +MAX_RETRIES_DEFAULT = 11 +BACKOFF_FACTOR = 1.0 + + +def _get_backoff_time(retry_count): + return BACKOFF_FACTOR * (2 ** retry_count) + + +class RetryRequests: + def __init__(self, max_requests_retries=MAX_RETRIES_DEFAULT) -> None: + self.max_retries = max_requests_retries + + def _retry_request(self, request_func, url, *args, **kwargs): + response = request_func(url, *args, **kwargs) + for retry_count in range(self.max_retries - 1): + if response.status_code == 501 or response.status_code < 500: + return response + time.sleep(_get_backoff_time(retry_count)) + response = request_func(url, **kwargs) + return response + + def get(self, url, *args, **kwargs): + return self._retry_request(requests.get, url, *args, **kwargs) + + def post(self, url, *args, **kwargs): + return self._retry_request(requests.post, url, *args, **kwargs) + + def put(self, url, *args, **kwargs): + return self._retry_request(requests.put, url, *args, **kwargs) + + def delete(self, url, *args, **kwargs): + return self._retry_request(requests.delete, url, *args, **kwargs) diff --git a/tests/mocks/test_retry.py b/tests/mocks/test_retry.py new file mode 100644 index 0000000..b6b6082 --- /dev/null +++ b/tests/mocks/test_retry.py @@ -0,0 +1,169 @@ +""" +Test that requests are retried if the server is not available. +""" +import unittest +from unittest.mock import patch + +import requests +import responses + +from kbcstorage.base import Endpoint +from kbcstorage.tables import Tables + +from .table_responses import list_response + + +class RetriesEndpoint(Endpoint): + def __init__(self, root_url, token, retries_count): + super().__init__(root_url, 'retries', token, max_requests_retries=retries_count) + + def list(self): + return self._get(self.base_url) + + +class TestRequestRetry(unittest.TestCase): + """ + Test that requests are retried. + """ + def setUp(self): + token = 'dummy_token' + base_url = 'https://connection.keboola.com/' + self.tables = Tables(base_url, token) + self.no_retries = RetriesEndpoint(base_url, token, 0) + self.two_retries = RetriesEndpoint(base_url, token, 2) + + @responses.activate + @patch('time.sleep', return_value=None) + def test_ok(self, sleep_mock): + """ + Retry will try at least 5 times. + """ + for _ in range(4): + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/tables', + json=list_response, + status=502 + ) + ) + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/tables', + json=list_response, + ) + ) + tables_list = self.tables.list() + assert isinstance(tables_list, list) + + @responses.activate + @patch('time.sleep', return_value=None) + def test_raises_error_many_tries(self, sleep_mock): + """ + Retry will fail if it gets enough of error responses. + """ + for _ in range(20): + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/tables', + json=list_response, + status=502 + ) + ) + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/tables', + json=list_response, + ) + ) + with self.assertRaises(requests.exceptions.HTTPError): + self.tables.list() + + @responses.activate + @patch('time.sleep', return_value=None) + def test_raises_error_on_4xx(self, sleep_mock): + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/tables', + json=list_response, + status=401 + ) + ) + with self.assertRaises(requests.exceptions.HTTPError): + self.tables.list() + + @responses.activate + @patch('time.sleep', return_value=None) + def test_no_retries(self, sleep_mock): + """ + Request wont be retried for endpoints that configured it. + """ + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/retries', + json=list_response, + status=502 + ) + ) + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/retries', + json=list_response, + ) + ) + with self.assertRaises(requests.exceptions.HTTPError): + self.no_retries.list() + + @responses.activate + @patch('time.sleep', return_value=None) + def test_two_retries_fail(self, sleep_mock): + """ + Request wont be retried for endpoints that configured it. + """ + for i in range(3): + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/retries', + json=list_response, + status=502 + ) + ) + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/retries', + json=list_response, + ) + ) + with self.assertRaises(requests.exceptions.HTTPError): + self.two_retries.list() + + @responses.activate + @patch('time.sleep', return_value=None) + def test_two_retries_ok(self, sleep_mock): + """ + Request wont be retried for endpoints that configured it. + """ + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/retries', + json=list_response, + status=502 + ) + ) + responses.add( + responses.Response( + method='GET', + url='https://connection.keboola.com/v2/storage/retries', + json=list_response, + ) + ) + assert isinstance(self.two_retries.list(), list)