From bd8ee72fca080b254a29c461a1f77951c73fb935 Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sat, 28 Jan 2017 13:59:21 -0500 Subject: [PATCH 1/8] clean up the code, add docstrings, and get rid of hacky async --- closeio_api/__init__.py | 207 ++++++++++++++++++++-------------------- 1 file changed, 106 insertions(+), 101 deletions(-) diff --git a/closeio_api/__init__.py b/closeio_api/__init__.py index c637edf..94db45a 100644 --- a/closeio_api/__init__.py +++ b/closeio_api/__init__.py @@ -1,20 +1,24 @@ -#!/usr/bin/env python -# -*- coding: utf-8 -*- - import json import time + import requests + from closeio_api.utils import local_tz_offset 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) @@ -23,61 +27,62 @@ 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 _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 -----------')) - - def dispatch(self, method_name, endpoint, **kwargs): - api_key = kwargs.pop('api_key', None) - data = kwargs.pop('data', None) - debug = kwargs.pop('debug', False) + self.session.headers.update({ + 'Content-Type': 'application/json', + 'X-TZ-Offset': self.tz_offset + }) + + 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 = self.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) + response = self.session.send(prepped_req, verify=self.verify) except requests.exceptions.ConnectionError: if (retry_count + 1 == self.max_retries): raise @@ -85,74 +90,74 @@ def dispatch(self, method_name, endpoint, **kwargs): else: 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(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 @@ -161,5 +166,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) + From 02bad5b4dbf4aee876a3240260f22b1880553b09 Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Tue, 21 Feb 2017 13:19:24 -0500 Subject: [PATCH 2/8] clean up grequests --- requirements.txt | 3 --- setup.py | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/requirements.txt b/requirements.txt index b225ca6..be282ab 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,5 @@ blist==1.3.6 click==6.6 -gevent==1.1.2 -greenlet==0.4.10 -grequests==0.3.0 progressbar-latest==2.4 python-dateutil==2.5.3 requests==2.11.1 diff --git a/setup.py b/setup.py index fc6f408..b59c8c5 100644 --- a/setup.py +++ b/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", From 3bfead9b6617ad87680f8d42ef17c4e4f1352c5d Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sat, 6 May 2017 19:30:57 -0400 Subject: [PATCH 3/8] basic tests --- requirements.txt | 2 ++ tests/__init__.py | 0 tests/test_api.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 tests/__init__.py create mode 100644 tests/test_api.py diff --git a/requirements.txt b/requirements.txt index 4661030..108a8f3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,3 @@ +pytest==3.0.7 requests==2.11.1 +responses==0.5.1 diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_api.py b/tests/test_api.py new file mode 100644 index 0000000..411e3e4 --- /dev/null +++ b/tests/test_api.py @@ -0,0 +1,53 @@ +import json + +import pytest +import responses + +from closeio_api import Client + + +@pytest.fixture +def api_client(): + return Client('fake-api-key') + + +@responses.activate +def test_list_leads(api_client): + responses.add( + responses.GET, + 'https://app.close.io/api/v1/lead/', + body=json.dumps({ + 'has_more': False, + 'data': [ + { + 'name': 'Sample Lead', + 'contacts': [], + # Other lead fields omitted for brevity + } + ] + }), + status=200, + content_type='application/json' + ) + + resp = api_client.get('lead') + assert not resp['has_more'] + assert resp['data'][0]['name'] == 'Sample Lead' + +@responses.activate +def test_fetch_lead(api_client): + responses.add( + responses.GET, + 'https://app.close.io/api/v1/lead/lead_abcdefghijklmnop/', + body=json.dumps({ + 'name': 'Sample Lead', + 'contacts': [], + # Other lead fields omitted for brevity + }), + status=200, + content_type='application/json' + ) + + resp = api_client.get('lead/lead_abcdefghijklmnop') + assert resp['name'] == 'Sample Lead' + From 285009546b67711f7a7b39287b807ec70f4e212e Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sat, 6 May 2017 22:31:29 -0400 Subject: [PATCH 4/8] circle.yml indicating that we should run pytest --- circle.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 circle.yml diff --git a/circle.yml b/circle.yml new file mode 100644 index 0000000..580d7af --- /dev/null +++ b/circle.yml @@ -0,0 +1,3 @@ +test: + override: + - pytest From cf72e73e553085e090efc015d4b3ab8acbb80d11 Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sat, 6 May 2017 22:33:21 -0400 Subject: [PATCH 5/8] specify the tests path --- setup.cfg | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 setup.cfg diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 0000000..5cc2b47 --- /dev/null +++ b/setup.cfg @@ -0,0 +1,2 @@ +[tool:pytest] +testpaths=tests From 9e403fff73feb731dfede99ce9be923a682b882a Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sat, 6 May 2017 23:08:00 -0400 Subject: [PATCH 6/8] test lead creation, search, rate limiting, and failed requests --- tests/test_api.py | 117 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 411e3e4..b20e690 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -3,7 +3,7 @@ import pytest import responses -from closeio_api import Client +from closeio_api import Client, APIError @pytest.fixture @@ -51,3 +51,118 @@ def test_fetch_lead(api_client): resp = api_client.get('lead/lead_abcdefghijklmnop') assert resp['name'] == 'Sample Lead' +@responses.activate +def test_create_lead(api_client): + def request_callback(request): + payload = json.loads(request.body) + expected_payload = {'name': 'Sample Lead'} + assert payload == expected_payload + return (200, {}, json.dumps(payload)) + + responses.add_callback( + responses.POST, + 'https://app.close.io/api/v1/lead/', + callback=request_callback, + content_type='application/json', + ) + + resp = api_client.post('lead', {'name': 'Sample Lead'}) + assert resp['name'] == 'Sample Lead' + +@responses.activate +def test_failed_create_lead(api_client): + responses.add( + responses.POST, + 'https://app.close.io/api/v1/lead/', + body='Forbidden', + status=403, + content_type='application/json' + ) + + with pytest.raises(APIError): + resp = api_client.post('lead', {'name': 'Sample Lead'}) + +@responses.activate +def test_search_for_leads(api_client): + responses.add( + responses.GET, + 'https://app.close.io/api/v1/lead/', + body=json.dumps({ + 'has_more': False, + 'data': [ + { + 'name': 'Sample Lead', + 'contacts': [], + # Other lead fields omitted for brevity + } + ] + }), + status=200, + content_type='application/json' + ) + def request_callback(request): + args = json.loads(request.args) + expected_args = {'query': 'name:sample'} + assert args == expected_args + return (200, {}, json.dumps({ + 'has_more': False, + 'data': [ + { + 'name': 'Sample Lead', + 'contacts': [], + # Other lead fields omitted for brevity + } + ] + })) + + responses.add_callback( + responses.GET, + 'https://app.close.io/api/v1/lead/', + callback=request_callback, + content_type='application/json', + ) + + resp = api_client.get('lead', params={'query': 'name:sample'}) + assert not resp['has_more'] + assert resp['data'][0]['name'] == 'Sample Lead' + +@responses.activate +def test_retry_on_rate_limit(api_client): + + with responses.RequestsMock() as rsps: + + # Rate limit the first request, suggesting that it can be retried in 1s. + rsps.add( + responses.GET, + 'https://app.close.io/api/v1/lead/lead_abcdefghijklmnop/', + body=json.dumps({ + "error": { + "rate_reset": 1, + "message": "API call count exceeded for this 15 second window", + "rate_limit": 600, + "rate_window": 15 + } + }), + status=429, + content_type='application/json' + ) + + # Respond correctly to the second request. + rsps.add( + responses.GET, + 'https://app.close.io/api/v1/lead/lead_abcdefghijklmnop/', + body=json.dumps({ + 'name': 'Sample Lead', + 'contacts': [], + # Other lead fields omitted for brevity + }), + status=200, + content_type='application/json' + ) + + resp = api_client.get('lead/lead_abcdefghijklmnop') + assert resp['name'] == 'Sample Lead' + + # Make sure two calls were made to the API (one rate limited and one + # successful). + assert len(rsps.calls) == 2 From d44a0549a2e782d7d5d1a5345515c693b67e157a Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sat, 6 May 2017 23:14:13 -0400 Subject: [PATCH 7/8] cleanup --- tests/test_api.py | 60 ++++++++++++++--------------------------------- 1 file changed, 18 insertions(+), 42 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index b20e690..c1876d8 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -6,26 +6,29 @@ from closeio_api import Client, APIError +SAMPLE_LEAD_RESPONSE = { + 'name': 'Sample Lead', + 'contacts': [], + # Other lead fields omitted for brevity +} + +SAMPLE_LEADS_RESPONSE = { + 'has_more': False, + 'data': [SAMPLE_LEAD_RESPONSE] +} + + @pytest.fixture def api_client(): + """Return the Close.io API client fixture.""" return Client('fake-api-key') - @responses.activate def test_list_leads(api_client): responses.add( responses.GET, 'https://app.close.io/api/v1/lead/', - body=json.dumps({ - 'has_more': False, - 'data': [ - { - 'name': 'Sample Lead', - 'contacts': [], - # Other lead fields omitted for brevity - } - ] - }), + body=json.dumps(SAMPLE_LEADS_RESPONSE), status=200, content_type='application/json' ) @@ -39,11 +42,7 @@ def test_fetch_lead(api_client): responses.add( responses.GET, 'https://app.close.io/api/v1/lead/lead_abcdefghijklmnop/', - body=json.dumps({ - 'name': 'Sample Lead', - 'contacts': [], - # Other lead fields omitted for brevity - }), + body=json.dumps(SAMPLE_LEAD_RESPONSE), status=200, content_type='application/json' ) @@ -84,26 +83,8 @@ def test_failed_create_lead(api_client): @responses.activate def test_search_for_leads(api_client): - responses.add( - responses.GET, - 'https://app.close.io/api/v1/lead/', - body=json.dumps({ - 'has_more': False, - 'data': [ - { - 'name': 'Sample Lead', - 'contacts': [], - # Other lead fields omitted for brevity - } - ] - }), - status=200, - content_type='application/json' - ) def request_callback(request): - args = json.loads(request.args) - expected_args = {'query': 'name:sample'} - assert args == expected_args + assert request.url == 'https://app.close.io/api/v1/lead/?query=name%3Asample' return (200, {}, json.dumps({ 'has_more': False, 'data': [ @@ -128,10 +109,9 @@ def request_callback(request): @responses.activate def test_retry_on_rate_limit(api_client): - with responses.RequestsMock() as rsps: - # Rate limit the first request, suggesting that it can be retried in 1s. + # Rate limit the first request and suggest it can be retried in 1 sec. rsps.add( responses.GET, 'https://app.close.io/api/v1/lead/lead_abcdefghijklmnop/', @@ -151,11 +131,7 @@ def test_retry_on_rate_limit(api_client): rsps.add( responses.GET, 'https://app.close.io/api/v1/lead/lead_abcdefghijklmnop/', - body=json.dumps({ - 'name': 'Sample Lead', - 'contacts': [], - # Other lead fields omitted for brevity - }), + body=json.dumps(SAMPLE_LEAD_RESPONSE), status=200, content_type='application/json' ) From ba3bfe4c7f8f279d44bd3d9d96f94ce12b657ca8 Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Sun, 7 May 2017 17:02:20 -0400 Subject: [PATCH 8/8] test a ValidationError --- tests/test_api.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index c1876d8..0d50c64 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -3,7 +3,7 @@ import pytest import responses -from closeio_api import Client, APIError +from closeio_api import Client, APIError, ValidationError SAMPLE_LEAD_RESPONSE = { @@ -142,3 +142,25 @@ def test_retry_on_rate_limit(api_client): # Make sure two calls were made to the API (one rate limited and one # successful). assert len(rsps.calls) == 2 + +@responses.activate +def test_validation_error(api_client): + responses.add( + responses.POST, + 'https://app.close.io/api/v1/contact/', + body=json.dumps({ + 'errors': [], + 'field-errors': { + 'lead': 'This field is required.' + } + }), + status=400, + content_type='application/json' + ) + + with pytest.raises(APIError) as excinfo: + api_client.post('contact', {'name': 'new lead'}) + + err = excinfo.value + assert err.errors == [] + assert err.field_errors['lead'] == 'This field is required.'