Clean up the code, add docstrings, and get rid of the poorly implemented async client

circle.yml

@@ -0,0 +1,3 @@
+test:
+  override:
+    - pytest

closeio_api/__init__.py

@@ -1,21 +1,27 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
 import logging
 import time
+
 import requests
+
 from closeio_api.utils import local_tz_offset
 
+
 DEFAULT_RATE_LIMIT_DELAY = 2   # Seconds
 
+
 class APIError(Exception):
+    """Raised when sending a request to the API failed."""
+
     def __init__(self, response):
         # For compatibility purposes we can access the original string through
         # the args property.
         super(APIError, self).__init__(response.text)
         self.response = response
 
+
 class ValidationError(APIError):
+    """Raised when the API returns validation errors."""
+
     def __init__(self, response):
         super(ValidationError, self).__init__(response)
 
@@ -24,154 +30,155 @@ def __init__(self, response):
         self.errors = data.get('errors', [])
         self.field_errors = data.get('field-errors', {})
 
+
 class API(object):
-    def __init__(self, base_url, api_key=None, tz_offset=None,
-                 async=False, max_retries=5, verify=True):
+    """Main class interacting with the Close.io API."""
+
+    def __init__(self, base_url, api_key=None, tz_offset=None, max_retries=5,
+                 verify=True):
         assert base_url
         self.base_url = base_url
-        self.async = async
         self.max_retries = max_retries
         self.tz_offset = tz_offset or str(local_tz_offset())
         self.verify = verify
 
-        if async:
-            import grequests
-            self.requests = grequests
-        else:
-            self.requests = requests
-
-        self.session = self.requests.Session()
+        self.session = requests.Session()
         if api_key:
             self.session.auth = (api_key, '')
-        self.session.headers.update({'Content-Type': 'application/json', 'X-TZ-Offset': self.tz_offset})
 
-    def _get_rate_limit_sleep_time(self, response):
-        """Get rate limit window expiration time from response."""
-
-        try:
-            data = response.json()
-            return float(data['error']['rate_reset'])
-        except (AttributeError, KeyError, ValueError):
-            logging.exception('Error parsing rate limiting response')
-            return DEFAULT_RATE_LIMIT_DELAY
-
-    def _print_request(self, request):
-        print('{}\n{}\n{}\n\n{}\n{}'.format(
-            '----------- HTTP Request -----------',
-            request.method + ' ' + request.url,
-            '\n'.join('{}: {}'.format(k, v) for k, v in request.headers.items()),
-            request.body or '',
-            '----------- /HTTP Request -----------'))
+        self.session.headers.update({
+            'Content-Type': 'application/json',
+            'X-TZ-Offset': self.tz_offset
+        })
 
-    def dispatch(self, method_name, endpoint, **kwargs):
-        api_key = kwargs.pop('api_key', None)
-        data = kwargs.pop('data', None)
-        debug = kwargs.pop('debug', False)
+    def _prepare_request(self, method_name, endpoint, api_key=None, data=None,
+                         debug=False, **kwargs):
+        """Construct and return a requests.Request object based on
+        provided parameters.
+        """
+        if api_key:
+            auth = (api_key, '')
+        else:
+            auth = None
+            assert self.session.auth, 'Must specify api_key.'
+
+        kwargs.update({
+            'auth': auth,
+            'json': data
+        })
+        request = requests.Request(method_name, self.base_url + endpoint,
+                                   **kwargs)
+        prepped_request = self.session.prepare_request(request)
+
+        if debug:
+            self._print_request(prepped_request)
+
+        return prepped_request
+
+    def _dispatch(self, method_name, endpoint, api_key=None, data=None,
+                  debug=False, **kwargs):
+        """Prepare and send a request with given parameters. Return a
+        dict containing the response data or raise an exception if any
+        errors occured.
+        """
+        prepped_req = self._prepare_request(method_name, endpoint, api_key,
+                                            data, debug, **kwargs)
 
         for retry_count in range(self.max_retries):
             try:
-                if api_key:
-                    auth = (api_key, '')
-                else:
-                    auth = None
-                    assert self.session.auth, 'Must specify api_key.'
-                kwargs.update({
-                    'auth': auth,
-                    'json': data
-                })
-                request = requests.Request(
-                    method_name,
-                    self.base_url+endpoint,
-                    **kwargs
-                )
-                prepped_request = self.session.prepare_request(request)
-                if debug:
-                    self._print_request(prepped_request)
-                response = self.session.send(prepped_request,
-                                             verify=self.verify)
-
-                # Check if request was rate limited
+                response = self.session.send(prepped_req, verify=self.verify)
+            except requests.exceptions.ConnectionError:
+                if retry_count + 1 == self.max_retries:
+                    raise
+                time.sleep(2)
+            else:
+                # Check if request was rate limited.
                 if response.status_code == 429:
                     sleep_time = self._get_rate_limit_sleep_time(response)
                     logging.debug('Request was rate limited, sleeping %d seconds', sleep_time)
                     time.sleep(sleep_time)
                     continue
 
-            except requests.exceptions.ConnectionError:
-                if (retry_count + 1 == self.max_retries):
-                    raise
-                time.sleep(2)
-            else:
+                # Break out of the retry loop if the request was successful.
                 break
 
-        if self.async:
-            return response
+        if response.ok:
+            return response.json()
+        elif response.status_code == 400:
+            raise ValidationError(response)
         else:
-            if response.ok:
-                return response.json()
-            elif response.status_code == 400:
-                raise ValidationError(response)
-            else:
-                raise APIError(response)
+            raise APIError(response)
+
+    def _get_rate_limit_sleep_time(self, response):
+        """Get rate limit window expiration time from response."""
+        try:
+            data = response.json()
+            return float(data['error']['rate_reset'])
+        except (AttributeError, KeyError, ValueError):
+            logging.exception('Error parsing rate limiting response')
+            return DEFAULT_RATE_LIMIT_DELAY
 
     def get(self, endpoint, params=None, **kwargs):
+        """Send a GET request to a given endpoint, for example:
+
+        >>> api.get('lead', {'query': 'status:"Potential"'})
+        {
+            'has_more': False,
+            'total_results': 5,
+            'data': [
+                # ... list of leads in "Potential" status
+            ]
+        }
+        """
         kwargs.update({'params': params})
-        return self.dispatch('get', endpoint+'/', **kwargs)
+        return self._dispatch('get', endpoint+'/', **kwargs)
 
     def post(self, endpoint, data, **kwargs):
+        """Send a POST request to a given endpoint, for example:
+
+        >>> api.post('lead', {'name': 'Brand New Lead'})
+        {
+            'name': 'Brand New Lead'
+            # ... rest of the response omitted for brevity
+        }
+        """
         kwargs.update({'data': data})
-        return self.dispatch('post', endpoint+'/', **kwargs)
+        return self._dispatch('post', endpoint+'/', **kwargs)
 
     def put(self, endpoint, data, **kwargs):
+        """Send a PUT request to a given endpoint, for example:
+
+        >>> api.put('lead/SOME_LEAD_ID', {'name': 'New Name'})
+        {
+            'name': 'New Name'
+            # ... rest of the response omitted for brevity
+        }
+        """
         kwargs.update({'data': data})
-        return self.dispatch('put', endpoint+'/', **kwargs)
+        return self._dispatch('put', endpoint+'/', **kwargs)
 
     def delete(self, endpoint, **kwargs):
-        return self.dispatch('delete', endpoint+'/', **kwargs)
-
-    # Only for async requests
-    def map(self, reqs, max_retries=None):
-        if max_retries is None:
-            max_retries = self.max_retries
-        # TODO
-        # There is no good way of catching or dealing with exceptions that are
-        # raised during the request sending process when using map or imap.
-        # When this issue is closed:
-        # https://github.com/kennethreitz/grequests/pull/15
-        # modify this method to repeat only the requests that failed because of
-        # connection errors
-        if self.async:
-            import grequests
-            responses = [(
-                response.json() if response.ok else APIError(response)
-            ) for response in grequests.map(reqs)]
-            # retry the api calls that failed until they succeed or the
-            # max_retries limit is reached
-            retries = 0
-            while True and retries < max_retries:
-                n_errors = sum([int(isinstance(response, APIError))
-                                for response in responses])
-                if not n_errors:
-                    break
-                # sleep 2 seconds before retrying requests
-                time.sleep(2)
-                error_ids = [i for i, resp in enumerate(responses)
-                             if isinstance(responses[i], APIError)]
-                new_reqs = [reqs[i] for i in range(len(responses))
-                            if i in error_ids]
-                new_resps = [(
-                    response.json() if response.ok else APIError(response)
-                ) for response in grequests.map(new_reqs)]
-                # update the responses that previously finished with errors
-                for i in range(len(error_ids)):
-                    responses[error_ids[i]] = new_resps[i]
-                retries += 1
-            return responses
+        """Send a DELETE request to a given endpoint, for example:
+
+        >>> api.delete('lead/SOME_LEAD_ID')
+        {}
+        """
+        return self._dispatch('delete', endpoint+'/', **kwargs)
+
+    def _print_request(self, req):
+        """Print a human-readable representation of a request."""
+        print('{}\n{}\n{}\n\n{}\n{}'.format(
+            '----------- HTTP Request -----------',
+            req.method + ' ' + req.url,
+            '\n'.join('{}: {}'.format(k, v) for k, v in req.headers.items()),
+            req.body or '',
+            '----------- /HTTP Request -----------'
+        ))
 
 
 class Client(API):
-    def __init__(self, api_key=None, tz_offset=None, async=False,
-                 max_retries=5, development=False):
+    def __init__(self, api_key=None, tz_offset=None, max_retries=5,
+                 development=False):
         if development:
             base_url = 'https://local.close.io:5001/api/v1/'
             # See https://github.com/kennethreitz/requests/issues/2966
@@ -180,5 +187,5 @@ def __init__(self, api_key=None, tz_offset=None, async=False,
             base_url = 'https://app.close.io/api/v1/'
             verify = True
         super(Client, self).__init__(base_url, api_key, tz_offset=tz_offset,
-                                     async=async, max_retries=max_retries,
-                                     verify=verify)
+                                     max_retries=max_retries, verify=verify)
+

requirements.txt

@@ -1,2 +1,3 @@
-grequests==0.3.0
+pytest==3.0.7
 requests==2.11.1
+responses==0.5.1

setup.cfg

@@ -0,0 +1,2 @@
+[tool:pytest]
+testpaths=tests

setup.py

@@ -1,7 +1,5 @@
 from setuptools import setup
 
-requires = ['requests >= 2.11.1', 'grequests >= 0.3.0']
-
 setup(
     name="closeio",
     packages=['closeio_api'],
@@ -10,7 +8,9 @@
     long_description="Closeio Python library",
     author="Close.io Team",
     url="https://github.com/closeio/closeio-api/",
-    install_requires=requires,
+    install_requires=[
+        'requests >= 2.11.1'
+    ],
     classifiers=[
         "Programming Language :: Python",
         "Programming Language :: Python :: 2",