Retry http errors

.travis.yml

@@ -14,6 +14,6 @@ install:
 
 # command to run tests
 script:
-  - pip install pytest==4.0.2 pytest-voluptuous==1.1.0 # for testing
+  - pip install pytest==4.0.2 pytest-voluptuous==1.1.0 httpretty==0.9.6 # for testing
   - LOG_LEVEL=DEBUG python -m pytest --junit-xml=junit.xml -vv app/tests
   - LOG_LEVEL=DEBUG python app/demo.py

deepomatic/api/__init__.py

@@ -1 +1,3 @@
 from deepomatic.api.version import __version__
+
+__all__ = ["__version__"]

deepomatic/api/client.py

@@ -23,10 +23,11 @@
 """
 
 from deepomatic.api.http_helper import HTTPHelper
+from deepomatic.api.resources.account import Account
 from deepomatic.api.resources.network import Network
-from deepomatic.api.resources.recognition import RecognitionSpec, RecognitionVersion
+from deepomatic.api.resources.recognition import (RecognitionSpec,
+                                                  RecognitionVersion)
 from deepomatic.api.resources.task import Task
-from deepomatic.api.resources.account import Account
 
 
 class Client(object):
@@ -44,7 +45,7 @@ def __init__(self, *args, **kwargs):
                If it fails raise a `DeepomaticException`.
            :type api_key: string
            :param verify_ssl (optional): whether to ask `requests` to verify the TLS/SSL certificates.
-               Defaults to `None`. 
+               Defaults to `None`.
                If `None` try to get it from the `DEEPOMATIC_API_VERIFY_TLS` environment variable (`0`: False, `1`: True).
                If not found it is set to True.
            :type verify_ssl: bool
@@ -60,6 +61,14 @@ def __init__(self, *args, **kwargs):
            :param pool_maxsize (optional): Set `requests.adapters.HTTPAdapter.pool_maxsize` for concurrent calls.
                Defaults to 20.
            :type pool_maxsize: int
+           :param requests_timeout: timeout of each request.
+               Defaults to `http_helper.RequestsTimeout.FAST`.
+               More details in the `requests` documentation: https://2.python-requests.org/en/master/user/advanced/#timeouts
+           :type requests_timeout: float or tuple(float, float)
+           :param http_retry (optional): Customize the retry of http errors.
+               Defaults to `HTTPRetry()`. Check out `http_retry.HTTPRetry` documentation for more information about the parameters and default values.
+               If `None`, no retry will be done on errors.
+           :type http_retry: http_retry.HTTPRetry
 
            :return: :class:`Client` object
            :rtype: deepomatic.api.client.Client

deepomatic/api/exceptions.py

@@ -23,6 +23,7 @@
 """
 
 import json
+from tenacity import RetryError
 
 
 ###############################################################################
@@ -83,11 +84,23 @@ def get_task_id(self):
 ###############################################################################
 
 class TaskTimeout(DeepomaticException):
-    def __init__(self, task):
+    def __init__(self, task, retry_error=None):
         self.task = task
+        self.retry_error = retry_error
 
     def __str__(self):
         return "Timeout on task:\n{}".format(json.dumps(self.task))
 
     def get_task_id(self):
         return self.task['id']
+
+
+###############################################################################
+
+
+class HTTPRetryError(RetryError):
+    pass
+
+
+class TaskRetryError(RetryError):
+    pass

deepomatic/api/http_helper.py

@@ -22,28 +22,50 @@
 THE SOFTWARE.
 """
 
-import os
+import functools
 import json
-import requests
-import sys
+import os
 import platform
-from requests.structures import CaseInsensitiveDict
-from six import string_types
+import sys
 
-from deepomatic.api.exceptions import DeepomaticException, BadStatus
+import requests
+from deepomatic.api.exceptions import BadStatus, DeepomaticException
+from deepomatic.api.http_retry import HTTPRetry
 from deepomatic.api.version import __title__, __version__
+from requests.structures import CaseInsensitiveDict
+from six import string_types
 
 API_HOST = 'https://api.deepomatic.com'
 API_VERSION = 0.7
 
 ###############################################################################
 
 
+class RequestsTimeout(object):
+    FAST = (3.05, 10.)
+    MEDIUM = (3.05, 60.)
+    SLOW = (3.05, 600.)
+
+
 class HTTPHelper(object):
-    def __init__(self, app_id=None, api_key=None, verify_ssl=None, host=None, version=API_VERSION, check_query_parameters=True, user_agent_prefix='', user_agent_suffix='', pool_maxsize=20):
+    def __init__(self, app_id=None, api_key=None, verify_ssl=None,
+                 host=None, version=API_VERSION, check_query_parameters=True,
+                 user_agent_prefix='', user_agent_suffix='', pool_maxsize=20,
+                 requests_timeout=RequestsTimeout.FAST, **kwargs):
         """
-        Init the HTTP helper with API key and secret
+        Init the HTTP helper with API key and secret.
+        Check out the `client.Client` documentation for more details about the parameters.
         """
+
+        # `http_retry` is retrieved from `kwargs` because a default parameter `http_retry=HTTPRetry()` is dangerous
+        # If the rest of the code mutates `self.http_retry`, it would change the default parameter for all other `Client` instances
+        self.http_retry = kwargs.pop('http_retry', HTTPRetry())
+
+        if len(kwargs) > 0:
+            raise TypeError("Too many parameters. HTTPRetry does not handle kwargs: {}".format(kwargs))
+
+        self.requests_timeout = requests_timeout
+
         if host is None:
             host = os.getenv('DEEPOMATIC_API_URL', API_HOST)
         if verify_ssl is None:
@@ -157,7 +179,25 @@ def recursive_json_dump(prefix, obj, data_dict, omit_dot=False):
 
         return new_dict
 
-    def make_request(self, func, resource, params=None, data=None, content_type='application/json', files=None, stream=False, *args, **kwargs):
+    def send_request(self, requests_callable, *args, **kwargs):
+        # requests_callable must be a method from the requests module
+
+        # this is the timeout of requests module
+        requests_timeout = kwargs.pop('timeout', self.requests_timeout)
+        http_retry = kwargs.pop('http_retry', self.http_retry)
+
+        functor = functools.partial(requests_callable, *args,
+                                    verify=self.verify,
+                                    timeout=requests_timeout, **kwargs)
+
+        if http_retry is not None:
+            return http_retry.retry(functor)
+
+        return functor()
+
+    def make_request(self, func, resource, params=None, data=None,
+                     content_type='application/json', files=None,
+                     stream=False, *args, **kwargs):
 
         if content_type is not None:
             if content_type.strip() == 'application/json':
@@ -204,7 +244,11 @@ def make_request(self, func, resource, params=None, data=None, content_type='app
 
         if not resource.startswith('http'):
             resource = self.resource_prefix + resource
-        response = func(resource, params=params, data=data, files=files, headers=headers, verify=self.verify, stream=stream, *args, **kwargs)
+
+        response = self.send_request(func, resource, *args,
+                                     params=params, data=data,
+                                     files=files, headers=headers,
+                                     stream=stream, **kwargs)
 
         # Close opened files
         for file in opened_files:

deepomatic/api/http_retry.py

@@ -0,0 +1,87 @@
+import functools
+
+from deepomatic.api import utils
+from deepomatic.api.exceptions import HTTPRetryError
+from requests.exceptions import (ProxyError, RequestException,
+                                 TooManyRedirects, URLRequired)
+from tenacity import (retry_if_exception, retry_if_result, stop_after_delay,
+                      wait_chain, wait_fixed, wait_random_exponential)
+
+
+class retry_if_exception_type(retry_if_exception):
+    # Taken from https://github.com/jd/tenacity/blob/2775f13b34b3ec67a774061a77fcd4e1e9b4157c/tenacity/retry.py#L72
+    # Extented to support blacklist types
+    def __predicate(self, e):
+        return (isinstance(e, self.exception_types) and
+                not isinstance(e, self.exception_types_blacklist))
+
+    def __init__(self, exception_types=Exception,
+                 exception_types_blacklist=()):
+        self.exception_types = exception_types
+        self.exception_types_blacklist = exception_types_blacklist
+        super(retry_if_exception_type, self).__init__(self.__predicate)
+
+
+class HTTPRetry(object):
+
+    """
+           :param retry_if (optional): predicate to retry on requests errors.
+               More details directly in tenacity source code:
+
+                   - https://github.com/jd/tenacity/blob/5.1.1/tenacity/__init__.py#L179
+                   - https://github.com/jd/tenacity/blob/5.1.1/tenacity/retry.py
+               If not provided, the default behavior is:
+                   - Retry on status code from Default.RETRY_STATUS_CODES
+                   - Retry on exceptions from Default.RETRY_EXCEPTION_TYPES excluding those from Default.RETRY_EXCEPTION_TYPES_BLACKLIST
+           :type retry_if: tenacity.retry_base
+           :param wait (optional): how to wait between retry
+               More details directly in tenacity source code https://github.com/jd/tenacity/blob/5.1.1/tenacity/wait.py
+
+               if not provided, the default behavior is:
+               ```
+                   random_wait = wait_random_exponential(multiplier=Default.RETRY_EXP_MULTIPLIER,
+                                                         max=Default.RETRY_EXP_MAX)
+                   wait_chain(wait_fixed(0.05),
+                              wait_fixed(0.1),
+                              wait_fixed(0.1) + random_wait)
+               ```
+            :type wait: tenacity.wait_base
+            :param stop (optional). Tell when to stop retrying. By default it stops retrying after a delay of 60 seconds. A last retry can be done just before this delay is reached, thus the total amount of elapsed time might be a bit higher. More details in tenacity source code https://github.com/jd/tenacity/blob/5.1.1/tenacity/stop.py
+                Raises tenacity.RetryError when timeout is reached.
+            :type timeout: tenacity.stop_base
+    """
+
+    class Default(object):
+        RETRY_EXP_MAX = 10.
+        RETRY_EXP_MULTIPLIER = 0.5
+        RETRY_STATUS_CODES = [500, 502, 503, 504]
+        RETRY_EXCEPTION_TYPES = (RequestException, )
+        RETRY_EXCEPTION_TYPES_BLACKLIST = (ValueError, ProxyError, TooManyRedirects, URLRequired)
+
+    def __init__(self, retry_if=None, wait=None, stop=None):
+        self.retry_status_code = {}
+        self.retry_if = retry_if
+        self.wait = wait
+        self.stop = stop
+
+        if self.stop is None:
+            self.stop = stop_after_delay(60)
+
+        if self.retry_if is None:
+            self.retry_status_code = set(HTTPRetry.Default.RETRY_STATUS_CODES)
+            self.retry_if = (retry_if_result(self.retry_if_status_code) |
+                             retry_if_exception_type(HTTPRetry.Default.RETRY_EXCEPTION_TYPES,
+                                                     HTTPRetry.Default.RETRY_EXCEPTION_TYPES_BLACKLIST))
+
+        if self.wait is None:
+            random_wait = wait_random_exponential(multiplier=HTTPRetry.Default.RETRY_EXP_MULTIPLIER,
+                                                  max=HTTPRetry.Default.RETRY_EXP_MAX)
+            self.wait = wait_chain(wait_fixed(0.05),
+                                   wait_fixed(0.1),
+                                   wait_fixed(0.1) + random_wait)
+
+    def retry(self, functor):
+        return utils.retry(functor, self.retry_if, self.wait, self.stop, retry_error_cls=HTTPRetryError)
+
+    def retry_if_status_code(self, response):
+        return response.status_code in self.retry_status_code

deepomatic/api/inference.py

@@ -0,0 +1,23 @@
+from deepomatic.api.exceptions import DeepomaticException
+from deepomatic.api.resources.task import Task
+from deepomatic.api.inputs import format_inputs
+
+
+class InferenceResource(object):
+    def inference(self, return_task=False, wait_task=True, **kwargs):
+        assert(self._pk is not None)
+
+        inputs = kwargs.pop('inputs', None)
+        if inputs is None:
+            raise DeepomaticException("Missing keyword argument: inputs")
+        content_type, data, files = format_inputs(inputs, kwargs)
+        result = self._helper.post(self._uri(pk=self._pk, suffix='/inference'), content_type=content_type, data=data, files=files)
+        task_id = result['task_id']
+        task = Task(self._helper, pk=task_id)
+        if wait_task:
+            task.wait()
+
+        if return_task:
+            return task
+        else:
+            return task['data']

deepomatic/api/mixins.py

@@ -86,6 +86,16 @@ def delete(self):
 
 class CreateableResource(object):
     def create(self, content_type='application/json', files=None, **kwargs):
+        post_kwargs = {}
+        # TODO: this is a hack, kwargs shouldn't be the data to post
+        # it should be the requests kwargs
+        # the fix will be a breaking change, so for now we just pop them
+        for key in ['http_retry', 'timeout']:
+            try:
+                post_kwargs[key] = kwargs.pop(key)
+            except KeyError:
+                pass
+
         if self._helper.check_query_parameters:
             for arg_name in kwargs:
                 if arg_name not in self.object_template or self.object_template[arg_name]._shoud_be_present_when_adding is False:
@@ -96,7 +106,10 @@ def create(self, content_type='application/json', files=None, **kwargs):
 
         if files is not None:
             content_type = 'multipart/mixed'
-        data = self._helper.post(self._uri(), data=kwargs, content_type=content_type, files=files)
+
+        data = self._helper.post(self._uri(), data=kwargs,
+                                 content_type=content_type,
+                                 files=files, **post_kwargs)
         return self.__class__(self._helper, pk=data['id'], data=data)
 
 
@@ -108,5 +121,3 @@ def list(self, offset=0, limit=100, **kwargs):
 
 
 ###############################################################################
-
-

deepomatic/api/resources/network.py

@@ -22,17 +22,19 @@
 THE SOFTWARE.
 """
 
-from six import string_types
 import numpy as np
-
+from deepomatic.api.http_helper import RequestsTimeout
+from deepomatic.api.inference import InferenceResource
+from deepomatic.api.mixins import (CreateableResource, DeletableResource,
+                                   ImmutableArg, ListableResource,
+                                   OptionnalArg, RequiredArg,
+                                   UpdatableResource)
 from deepomatic.api.resource import Resource
-from deepomatic.api.utils import InferenceResource
-from deepomatic.api.mixins import CreateableResource, ListableResource, UpdatableResource, DeletableResource
-from deepomatic.api.mixins import RequiredArg, OptionnalArg, ImmutableArg
-
+from six import string_types
 
 ###############################################################################
 
+
 class Network(ListableResource,
               CreateableResource,
               UpdatableResource,
@@ -66,6 +68,12 @@ def inference(self, convert_to_numpy=True, return_task=False, **kwargs):
         else:
             return result
 
+    def create(self, *args, **kwargs):
+        # No retry on Network.create() errors by default as this is a large request
+        kwargs['http_retry'] = kwargs.get('http_retry', None)
+        kwargs['timeout'] = kwargs.get('timeout', RequestsTimeout.SLOW)
+        return super(Network, self).create(*args, **kwargs)
+
     @staticmethod
     def _convert_result_to_numpy(result):
         new_result = {}

deepomatic/api/resources/recognition.py

@@ -25,7 +25,7 @@
 from six import string_types
 
 from deepomatic.api.resource import Resource, ResourceList
-from deepomatic.api.utils import InferenceResource
+from deepomatic.api.inference import InferenceResource
 from deepomatic.api.mixins import CreateableResource, ListableResource, UpdatableResource, DeletableResource
 from deepomatic.api.mixins import RequiredArg, OptionnalArg, ImmutableArg, UpdateOnlyArg
 
@@ -76,4 +76,3 @@ class RecognitionVersion(CreateableResource,
         'network_id':       RequiredArg(),
         'post_processings': RequiredArg(),
     }
-