oanda / oandapy

Python wrapper for the OANDA REST API
MIT License
321 stars 147 forks source link

Add a timeout for API.request() #44

Open Anjum48 opened 7 years ago

Anjum48 commented 7 years ago

Would it be worthwhile adding a timeout for API.request()?

At the moment, the default of None is used meaning that if no response is received, the code will just hang, and you might get a message like this: HTTPSConnectionPool(host='api-fxpractice.oanda.com', port=443): Read timed out. (read timeout=None)

Looking at the docs for requests, i'm not sure if the timeout can be set using the request_args, since timeout is a named argument (http://docs.python-requests.org/en/master/api/#requests.request). I tried passing the timeout in a dict inside the mixin methods like this params={"timeout": 0.001}, but that didn't seem to do anything - I might be wrong on this though.

One solution would be to add a timeout to the API class as follows, so that is is explicitly given to func(url, **request_args):

class API(EndpointsMixin, object):
    def __init__(self,
                 environment="practice", access_token=None, headers=None, timeout=10):
        """Instantiates an instance of OandaPy's API wrapper
        :param environment: (optional) Provide the environment for oanda's
         REST api, either 'sandbox', 'practice', or 'live'. Default: practice
        :param access_token: (optional) Provide a valid access token if you
         have one. This is required if the environment is not sandbox.
        """

        if environment == 'sandbox':
            self.api_url = 'http://api-sandbox.oanda.com'
        elif environment == 'practice':
            self.api_url = 'https://api-fxpractice.oanda.com'
        elif environment == 'live':
            self.api_url = 'https://api-fxtrade.oanda.com'
        else:
            raise BadEnvironment(environment)

        self.access_token = access_token
        self.client = requests.Session()
        self.timeout = timeout 

        # personal token authentication
        if self.access_token:
            self.client.headers['Authorization'] = 'Bearer '+self.access_token

        if headers:
            self.client.headers.update(headers)

    def request(self, endpoint, method='GET', params=None):
        """Returns dict of response from OANDA's open API
        :param endpoint: (required) OANDA API (e.g. v1/instruments)
        :type endpoint: string
        :param method: (optional) Method of accessing data, either GET or POST.
         (default GET)
        :type method: string
        :param params: (optional) Dict of parameters (if any) accepted the by
         OANDA API endpoint you are trying to access (default None)
        :type params: dict or None
        """

        url = '%s/%s' % (self.api_url, endpoint)

        method = method.lower()
        params = params or {}

        func = getattr(self.client, method)

        request_args = {}
        if method == 'get':
            request_args['params'] = params
        else:
            request_args['data'] = params

        try:
            response = func(url, self.timeout, **request_args)
            content = response.content.decode('utf-8')
        except requests.RequestException as e:
            print (str(e))
            content = dict(error=str(e))

        content = json.loads(content)

        # error message
        if response.status_code >= 400:
            raise OandaError(content)

        return content

This way, at least an exception will be raised preventing the code from hanging. I can set up a PR if required