Skip to content
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

Adding timeout to TwilioHttpClient constructor #485

Merged
merged 4 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ We'd like to thank the following people who have contributed to the
- Evan Cooke
- tysonholub
- Brodan
- Kyle Jones <kylejones1310@outlook.com>
60 changes: 56 additions & 4 deletions tests/unit/http/test_http_client.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
# -*- coding: utf-8 -*-

import six

import unittest

import mock
from mock import patch, Mock
from requests import Request
from requests import Session
from requests import Request, Session

from twilio.http.http_client import TwilioHttpClient
from twilio.http.response import Response
Expand Down Expand Up @@ -42,6 +39,61 @@ def test_request_sets_host_header_if_missing(self):
self.assertIsNotNone(self.client.last_request)
self.assertIsNotNone(self.client.last_response)

def test_request_with_timeout(self):
self.request_mock.url = 'https://api.twilio.com/'
self.request_mock.headers = {'Host': 'other.twilio.com'}

response = self.client.request(
'doesnt matter', 'doesnt matter', None, None, None, None, 30, None)

self.assertEqual('other.twilio.com', self.request_mock.headers['Host'])
self.assertEqual(200, response.status_code)
self.assertEqual('testing-unicode: Ω≈ç√, 💩', response.content)

def test_request_where_method_timeout_equals_zero(self):
self.request_mock.url = 'https://api.twilio.com/'
self.request_mock.headers = {'Host': 'other.twilio.com'}

try:
self.client.request(
'doesnt matter', 'doesnt matter', None, None, None, None, 0, None)
except Exception as e:
self.assertEqual(ValueError, type(e))

def test_request_where_class_timeout_manually_set(self):
self.request_mock.url = 'https://api.twilio.com/'
self.request_mock.headers = {'Host': 'other.twilio.com'}
self.client.timeout = 30

response = self.client.request(
'doesnt matter', 'doesnt matter')
self.assertEqual('other.twilio.com', self.request_mock.headers['Host'])
self.assertEqual(200, response.status_code)
self.assertEqual('testing-unicode: Ω≈ç√, 💩', response.content)

def test_request_where_class_timeout_equals_zero(self):
self.request_mock.url = 'https://api.twilio.com/'
self.request_mock.headers = {'Host': 'other.twilio.com'}
self.client.timeout = 0

try:
self.client.request(
'doesnt matter', 'doesnt matter')
except Exception as e:
self.assertEqual(type(e), ValueError)

def test_request_where_class_timeout_and_method_timeout_set(self):
self.request_mock.url = 'https://api.twilio.com/'
self.request_mock.headers = {'Host': 'other.twilio.com'}
self.client.timeout = 30

response = self.client.request(
'doesnt matter', 'doesnt matter', None, None, None, None, 15, None)

self.assertEqual('other.twilio.com', self.request_mock.headers['Host'])
self.assertEqual(200, response.status_code)
self.assertEqual('testing-unicode: Ω≈ç√, 💩', response.content)

def test_request_with_unicode_response(self):
self.request_mock.url = 'https://api.twilio.com/'
self.request_mock.headers = {'Host': 'other.twilio.com'}
Expand Down
18 changes: 16 additions & 2 deletions twilio/http/http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,24 @@ class TwilioHttpClient(HttpClient):
"""
General purpose HTTP Client for interacting with the Twilio API
"""
def __init__(self, pool_connections=True, request_hooks=None):
def __init__(self, pool_connections=True, request_hooks=None, timeout=None):
"""
Constructor for the TwilioHttpClient

:param bool pool_connections
:param request_hooks
:param int timeout: Timeout for the requests.
Timeout should never be zero (0) or less.
"""
self.session = Session() if pool_connections else None
self.last_request = None
self.last_response = None
self.request_hooks = request_hooks or hooks.default_hooks()

if timeout is not None and timeout <= 0:
raise ValueError(timeout)
self.timeout = timeout

def request(self, method, url, params=None, data=None, headers=None, auth=None, timeout=None,
allow_redirects=False):
"""
Expand All @@ -37,6 +49,8 @@ def request(self, method, url, params=None, data=None, headers=None, auth=None,
:return: An http response
:rtype: A :class:`Response <twilio.rest.http.response.Response>` object
"""
if timeout is not None and timeout <= 0:
raise ValueError(timeout)

kwargs = {
'method': method.upper(),
Expand Down Expand Up @@ -65,7 +79,7 @@ def request(self, method, url, params=None, data=None, headers=None, auth=None,
response = session.send(
prepped_request,
allow_redirects=allow_redirects,
timeout=timeout,
timeout=timeout if timeout is not None else self.timeout,
eshanholtz marked this conversation as resolved.
Show resolved Hide resolved
)

_logger.info('{method} Response: {status} {text}'.format(method=method, status=response.status_code, text=response.text))
Expand Down