-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TDL-16368 add request timeout #19
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
15750ea
TDL-16368 ad request timeout
namrata270998 6c78c91
added coverage
namrata270998 0c2e941
resolved pylint
namrata270998 cfe703a
added connectionerror in backoff
namrata270998 9cad972
updated readme
namrata270998 c0ff519
Update README.md
namrata270998 4ad5ca5
added factor in backoff
namrata270998 25f9bf0
Merge branch 'TDL-16368-implenet-request-timeout' of https://github.c…
namrata270998 acbcb56
resolved commets
namrata270998 f61529e
resolved comments
namrata270998 9e34a73
resolved comments
namrata270998 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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'}) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@namrata270998 Can we merge the above backoff with the already available backoff on
check_access_token
function for consistency?tap-recharge/tap_recharge/client.py
Lines 133 to 138 in acbcb56
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot merge the 2 backoffs as the
check_access_token()
is also called from therequest()
function which has a backoff. Hence, to avoid more than 5 times backoff, it is placed on the top of__enter__()