Skip to content

Commit

Permalink
TDL-16368 add request timeout (#19)
Browse files Browse the repository at this point in the history
* TDL-16368 ad request timeout

* added coverage

* resolved pylint

* added connectionerror in backoff

* updated readme

* Update README.md

Co-authored-by: Umang Agrawal <80704207+umangagrawal-crest@users.noreply.github.com>

* added factor in backoff

* resolved commets

* resolved comments

* resolved comments

Co-authored-by: Umang Agrawal <80704207+umangagrawal-crest@users.noreply.github.com>
  • Loading branch information
namrata270998 and umangagrawal-crest authored Dec 1, 2021
1 parent a3adb6d commit d195b4b
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 12 deletions.
13 changes: 12 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,18 @@ jobs:
name: 'pylint'
command: |
source /usr/local/share/virtualenvs/tap-recharge/bin/activate
pylint tap_recharge --disable 'missing-module-docstring,missing-function-docstring,missing-class-docstring,no-else-raise,raise-missing-from,inconsistent-return-statements'
pylint tap_recharge --disable 'missing-module-docstring,missing-function-docstring,missing-class-docstring,no-else-raise,raise-missing-from,inconsistent-return-statements,line-too-long'
- run:
name: 'Unit Tests'
command: |
source /usr/local/share/virtualenvs/tap-recharge/bin/activate
pip install nose coverage
nosetests --with-coverage --cover-erase --cover-package=tap_recharge --cover-html-dir=htmlcov tests/unittests
coverage html
- store_test_results:
path: test_output/report.xml
- store_artifacts:
path: htmlcov
- run:
when: always
name: 'Integration Tests'
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,12 @@ This tap:
{
"access_token": "YOUR_ACCESS_TOKEN",
"start_date": "2019-01-01T00:00:00Z",
"user_agent": "tap-recharge <api_user_email@your_company.com>"
"user_agent": "tap-recharge <api_user_email@your_company.com>",
"request_timeout": 300
}
```
Optionally, also create a `state.json` file. `currently_syncing` is an optional attribute used for identifying the last object to be synced in case the job is interrupted mid-stream. The next run would begin where the last job left off.
Optionally, also create a `state.json` file. `currently_syncing` is an optional attribute used for identifying the last object to be synced in case the job is interrupted mid-stream. The next run would begin where the last job left off. The `request_timeout` is an optional parameter to set a timeout for requests. Default: 300 seconds
```json
{
Expand Down
4 changes: 3 additions & 1 deletion tap_recharge/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ def main():

with RechargeClient(
parsed_args.config['access_token'],
parsed_args.config['user_agent']) as client:
parsed_args.config['user_agent'],
parsed_args.config.get('request_timeout')
) as client:

state = {}
if parsed_args.state:
Expand Down
31 changes: 23 additions & 8 deletions tap_recharge/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

import singer
from singer import metrics, utils
from requests.exceptions import Timeout

LOGGER = singer.get_logger()

REQUEST_TIMEOUT = 300

class Server5xxError(Exception):
pass
Expand Down Expand Up @@ -101,20 +102,34 @@ class RechargeClient:
def __init__(
self,
access_token,
user_agent=None):
user_agent=None,
request_timeout=REQUEST_TIMEOUT):
self.__access_token = access_token
self.__user_agent = user_agent
self.__session = requests.Session()
self.__base_url = None
self.__verified = False

# if request_timeout is other than 0,"0" or "" then use request_timeout
if request_timeout and float(request_timeout):
request_timeout = float(request_timeout)
else: # If value is 0,"0" or "" then set default to 300 seconds.
request_timeout = REQUEST_TIMEOUT
self.request_timeout = request_timeout

# Backoff the request for 5 times when Timeout or Connection error occurs
@backoff.on_exception(
backoff.expo,
(Timeout, requests.ConnectionError),
max_tries=5,
factor=2)
def __enter__(self):
self.__verified = self.check_access_token()
return self

def __exit__(self, exception_type, exception_value, traceback):
self.__session.close()

# Backoff the request for 5 times when Timeout error occurs
@backoff.on_exception(
backoff.expo,
Server5xxError,
Expand All @@ -131,17 +146,18 @@ def check_access_token(self):
response = self.__session.get(
# Simple endpoint that returns 1 record w/ default organization URN
url='https://api.rechargeapps.com',
headers=headers)
headers=headers,
timeout=self.request_timeout)
if response.status_code != 200:
LOGGER.error('Error status_code = %s', response.status_code)
raise_for_error(response)
else:
return True


# Added backoff for 5 times when Timeout error occurs
@backoff.on_exception(
backoff.expo,
(Server5xxError, requests.ConnectionError, Server429Error),
(Timeout, Server5xxError, requests.ConnectionError, Server429Error),
max_tries=5,
factor=2)
# Call/rate limit: https://developer.rechargepayments.com/#rate-limits
Expand Down Expand Up @@ -174,7 +190,7 @@ def request(self, method, path=None, url=None, **kwargs):
kwargs['headers']['Content-Type'] = 'application/json'

with metrics.http_request_timer(endpoint) as timer:
response = self.__session.request(method, url, stream=True, **kwargs)
response = self.__session.request(method, url, stream=True, timeout=self.request_timeout, **kwargs)
timer.tags[metrics.Tag.http_status_code] = response.status_code

if response.status_code >= 500:
Expand All @@ -192,7 +208,6 @@ def request(self, method, path=None, url=None, **kwargs):
except Exception as err:
LOGGER.error(err)
raise Exception(err)

return response_json, response.links

def get(self, path, **kwargs):
Expand Down
150 changes: 150 additions & 0 deletions tests/unittests/test_request_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
from tap_recharge.client import RechargeClient
import unittest
from unittest import mock
from requests.exceptions import Timeout, ConnectionError

class TestBackoffError(unittest.TestCase):
'''
Test that backoff logic works properly.
'''
@mock.patch('tap_recharge.client.requests.Session.request')
@mock.patch('tap_recharge.client.RechargeClient.check_access_token')
def test_request_timeout_and_backoff(self, mock_get_token, mock_request):
"""
Check whether the request backoffs properly for request() for 5 times in case of Timeout error.
"""
mock_request.side_effect = Timeout
client = RechargeClient("dummy_access_token", "dummy_user_agent", 300)
with self.assertRaises(Timeout):
client.request("GET")
self.assertEquals(mock_request.call_count, 5)

@mock.patch('tap_recharge.client.requests.Session.request')
def test_check_access_token_timeout_and_backoff(self, mocked_request):
"""
Check whether the request backoffs properly for __enter__() for 5 times in case of Timeout error.
"""
mocked_request.side_effect = Timeout

config = {
"access_token": "dummy_at",
"user_agent": "dummy_ua"
}
# initialize 'RechargeClient'
try:
with RechargeClient(
config['access_token'],
config['user_agent'],
config.get('request_timeout')) as client:
pass
except Timeout:
pass
# verify that we backoff for 5 times
self.assertEquals(mocked_request.call_count, 5)

@mock.patch('tap_recharge.client.requests.Session.request')
def test_check_access_token_connection_error_and_backoff(self, mocked_request):
"""
Check whether the request backoffs properly for __enter__() for 5 times in case of Timeout error.
"""
mocked_request.side_effect = ConnectionError

config = {
"access_token": "dummy_at",
"user_agent": "dummy_ua"
}
# initialize 'RechargeClient'
try:
with RechargeClient(
config['access_token'],
config['user_agent'],
config.get('request_timeout')) as client:
pass
except ConnectionError:
pass
# verify that we backoff for 5 times
self.assertEquals(mocked_request.call_count, 5)

class MockResponse():
'''
Mock response object for the requests call
'''
def __init__(self, resp, status_code, content=[""], headers=None, raise_error=False, text={}, links=""):
self.json_data = resp
self.status_code = status_code
self.content = content
self.headers = headers
self.raise_error = raise_error
self.text = text
self.reason = "error"
self.links = links

def prepare(self):
return (self.json_data, self.status_code, self.content, self.headers, self.raise_error)

def json(self):
return self.text

class TestRequestTimeoutValue(unittest.TestCase):
'''
Test that request timeout parameter works properly in various cases
'''
@mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200))
@mock.patch('tap_recharge.client.RechargeClient.check_access_token')
def test_config_provided_request_timeout(self, mock_get, mock_request):
"""
Unit tests to ensure that request timeout is set based on config value
"""
config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": 100}
client = RechargeClient(**config)
client.request("GET", "dummy_path")

mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=100.0, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'})

@mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200))
@mock.patch('tap_recharge.client.RechargeClient.check_access_token')
def test_default_value_request_timeout(self, mock_get, mock_request):
"""
Unit tests to ensure that request timeout is set based default value
"""
config = {"access_token": "dummy_at", "user_agent": "dummy_ua"}
client = RechargeClient(**config)
client.request("GET", "dummy_path")

mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=300, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'})

@mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200))
@mock.patch('tap_recharge.client.RechargeClient.check_access_token')
def test_config_provided_empty_request_timeout(self, mock_get, mock_request):
"""
Unit tests to ensure that request timeout is set based on default value if empty value is given in config
"""
config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": ""}
client = RechargeClient(**config)
client.request("GET", "dummy_path")

mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=300.0, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'})

@mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200))
@mock.patch('tap_recharge.client.RechargeClient.check_access_token')
def test_config_provided_string_request_timeout(self, mock_get, mock_request):
"""
Unit tests to ensure that request timeout is set based on config string value
"""
config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": "100"}
client = RechargeClient(**config)
client.request("GET", "dummy_path")

mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=100.0, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'})

@mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200))
@mock.patch('tap_recharge.client.RechargeClient.check_access_token')
def test_config_provided_float_request_timeout(self, mock_get, mock_request):
"""
Unit tests to ensure that request timeout is set based on config float value
"""
config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": 100.8}
client = RechargeClient(**config)
client.request("GET", "dummy_path")

mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=100.8, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'})

0 comments on commit d195b4b

Please sign in to comment.