From 2c9d613c0fa6ace9b7ecf58df538b65f84729874 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Thu, 23 Jul 2015 17:11:38 -0700
Subject: [PATCH 01/10] Progress adding connect_timeout

---
 bravado/client.py          | 18 ++++++++++++------
 bravado/fido_client.py     | 10 ++++++++--
 bravado/http_client.py     |  3 ++-
 bravado/requests_client.py | 38 ++++++++++++++++++++++++++++++++++++--
 4 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/bravado/client.py b/bravado/client.py
index fe0ace14..834f8194 100644
--- a/bravado/client.py
+++ b/bravado/client.py
@@ -174,8 +174,8 @@ def __getattr__(self, name):
 
     def construct_request(self, **op_kwargs):
         """
-        :param op_kwargs: parameter name/value pairs to pass to the invocation
-            of the operation.
+        :param op_kwargs: parameter name/value pairs to passed to the
+            invocation of the operation.
         :return: request in dict form
         """
         request_options = op_kwargs.pop('_request_options', {})
@@ -183,16 +183,22 @@ def construct_request(self, **op_kwargs):
         request = {
             'method': self.operation.http_method.upper(),
             'url': url,
-            'params': {},
+            'params': {},  # filled in downstream
             'headers': request_options.get('headers', {}),
         }
+
+        # Copy over optional request options
+        for request_option in ('connect_timeout', 'timeout'):
+            if request_option in request_options:
+                request[request_option] = request_options[request_option]
+
         self.construct_params(request, op_kwargs)
         return request
 
     def construct_params(self, request, op_kwargs):
         """
         Given the parameters passed to the operation invocation, validates and
-        marshals the parmameters into the provided request dict.
+        marshals the parameters into the provided request dict.
 
         :type request: dict
         :param op_kwargs: the kwargs passed to the operation invocation
@@ -225,10 +231,10 @@ def __call__(self, **op_kwargs):
         """
         log.debug(u"%s(%s)" % (self.operation.operation_id, op_kwargs))
         warn_for_deprecated_op(self.operation)
-        request = self.construct_request(**op_kwargs)
+        request_params = self.construct_request(**op_kwargs)
 
         def response_callback(response_adapter):
             return unmarshal_response(response_adapter, self)
 
         return self.operation.swagger_spec.http_client.request(
-            request, response_callback)
+            request_params, response_callback)
diff --git a/bravado/fido_client.py b/bravado/fido_client.py
index 55a4edae..ab39fbc4 100644
--- a/bravado/fido_client.py
+++ b/bravado/fido_client.py
@@ -59,13 +59,19 @@ def request(self, request_params, response_callback=None):
         url = '%s?%s' % (request_params['url'], urllib_utf8.urlencode(
             request_params.get('params', []), True))
 
-        request_params = {
+        fetch_kwargs = {
             'method': str(request_params.get('method', 'GET')),
             'body': stringify_body(request_params),
             'headers': request_params.get('headers', {}),
         }
 
-        return HttpFuture(fido.fetch(url, **request_params),
+        for fetch_kwarg in ('connect_timeout', 'timeout'):
+            if fetch_kwarg in request_params:
+                fetch_kwargs[fetch_kwargs] = request_params[fetch_kwarg]
+
+        concurrent_future = fido.fetch(url, **fetch_kwargs)
+
+        return HttpFuture(concurrent_future,
                           FidoResponseAdapter,
                           response_callback)
 
diff --git a/bravado/http_client.py b/bravado/http_client.py
index 07c238be..08fd4844 100644
--- a/bravado/http_client.py
+++ b/bravado/http_client.py
@@ -9,7 +9,8 @@ class HttpClient(object):
 
     def request(self, request_params, response_callback=None):
         """
-        :param request_params: complete request data.
+        :param request_params: complete request data. e.g. url, method,
+            headers, body, params, connect_timeout, timeout, etc.
         :type request_params: dict
         :param response_callback: Function to be called on response
         :type response_callback: method
diff --git a/bravado/requests_client.py b/bravado/requests_client.py
index bbf3c6d8..8bed2138 100644
--- a/bravado/requests_client.py
+++ b/bravado/requests_client.py
@@ -93,6 +93,31 @@ def __init__(self):
         self.session = requests.Session()
         self.authenticator = None
 
+    @staticmethod
+    def separate_params(request_params):
+        """Splits the passed in dict of request_params into two buckets.
+
+        - sanitized_params are valid kwargs for constructing a
+          requests.Request(..)
+        - misc_options are things like timeouts which can't be communicated
+          to the Requests library via the requests.Request(...) constructor.
+
+        :param request_params: kitchen sink of request params. Treated as a
+            read-only dict.
+        :returns: tuple(sanitized_params, misc_options)
+        """
+        sanitized_params = request_params.copy()
+        request_options = {}
+
+        if 'connect_timeout' in sanitized_params:
+            request_options['connect_timeout'] = \
+                sanitized_params.pop('connect_timeout')
+
+        if 'timeout' in sanitized_params:
+            request_options['timeout'] = sanitized_params.pop('timeout')
+
+        return sanitized_params, request_options
+
     def request(self, request_params, response_callback=None):
         """
         :param request_params: complete request data.
@@ -101,8 +126,12 @@ def request(self, request_params, response_callback=None):
         :returns: HTTP Future object
         :rtype: :class: `bravado_core.http_future.HttpFuture`
         """
+        sanitized_params, misc_options = self.separate_params(request_params)
+
         requests_future = RequestsFutureAdapter(
-            self.session, self.authenticated_request(request_params))
+            self.session,
+            self.authenticated_request(sanitized_params),
+            misc_options)
 
         return HttpFuture(
             requests_future,
@@ -170,14 +199,18 @@ def json(self, **kwargs):
 class RequestsFutureAdapter(object):
     """A future which inputs HTTP params"""
 
-    def __init__(self, session, request):
+    def __init__(self, session, request, misc_options):
         """Kicks API call for Requests client
 
         :param session: session object to use for making the request
         :param request: dict containing API request parameters
+        :param misc_options: misc options to apply when sending a HTTP request.
+            e.g. timeout, connect_timeout, etc
+        :type misc_options: dict
         """
         self.session = session
         self.request = request
+        self.misc_options = misc_options
 
     def check_for_exceptions(self, response):
         try:
@@ -193,6 +226,7 @@ def result(self, timeout):
         :return: raw response from the server
         :rtype: dict
         """
+        # TODO: left off here - passing timeout tuple to self.session.send(...)
         request = self.request
         response = self.session.send(
             self.session.prepare_request(request),

From 3e97ccf870b4cf4058ab88054c8125f474d9cda5 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Fri, 24 Jul 2015 14:31:21 -0700
Subject: [PATCH 02/10] Add passing timeout via _request_options to the
 Requests client

---
 bravado/requests_client.py                    | 42 +++++++++++++-
 .../build_timeout_test.py                     | 57 +++++++++++++++++++
 2 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100644 tests/requests_client/RequestsFutureAdapter/build_timeout_test.py

diff --git a/bravado/requests_client.py b/bravado/requests_client.py
index 8bed2138..ae1b9e34 100644
--- a/bravado/requests_client.py
+++ b/bravado/requests_client.py
@@ -218,19 +218,55 @@ def check_for_exceptions(self, response):
         except Exception as e:
             add_response_detail_to_errors(e)
 
+    def build_timeout(self, result_timeout):
+        """
+        Build the appropriate timeout object to pass to `session.send(...)`.
+
+        :param result_timeout: timeout that was passed into `result(..)`.
+        :return: timeout
+        :rtype: float or tuple(connect_timeout, timeout)
+        """
+        timeout = None
+        has_service_timeout = 'timeout' in self.misc_options
+        has_result_timeout = result_timeout is not None
+
+        service_timeout = self.misc_options.get('timeout')
+        if has_service_timeout and has_result_timeout:
+            # The API provides two ways to pass a timeout :( We're stuck
+            # dealing with it until we're ready to make a non-backwards
+            # compatible change.
+            timeout = service_timeout
+            if service_timeout != result_timeout:
+                timeout = max(service_timeout, result_timeout)
+                log.warn("Two different timeouts have been passed: "
+                         "_request_options['timeout'] = %s and "
+                         "future.result(timeout=%s). Using the highest "
+                         "timeout of %s."
+                         .format(service_timeout, result_timeout, timeout))
+        elif has_service_timeout:
+            timeout = service_timeout
+        elif has_result_timeout:
+            timeout = result_timeout
+
+        if 'connect_timeout' in self.misc_options:
+            # Requests is weird in that if you want to specify a
+            # connect_timeout and idle timeout, then a tuple of the two is
+            # required.
+            return self.misc_options['connect_timeout'], timeout
+        return timeout
+
     def result(self, timeout):
         """Blocking call to wait for API response
 
         :param timeout: timeout in seconds to wait for response
-        :type timeout: integer
+        :type timeout: float
         :return: raw response from the server
         :rtype: dict
         """
-        # TODO: left off here - passing timeout tuple to self.session.send(...)
         request = self.request
         response = self.session.send(
             self.session.prepare_request(request),
-            timeout=timeout)
+            timeout=self.build_timeout(timeout))
 
         self.check_for_exceptions(response)
 
diff --git a/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py b/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
new file mode 100644
index 00000000..0ca04506
--- /dev/null
+++ b/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
@@ -0,0 +1,57 @@
+from mock import Mock
+import pytest
+from requests.sessions import Session
+
+from bravado.requests_client import RequestsFutureAdapter
+
+
+@pytest.fixture
+def request():
+    return dict(url='http://foo.com')
+
+
+@pytest.fixture
+def session():
+    return Mock(spec=Session)
+
+
+def test_no_timeouts(session, request):
+    misc_options = {}
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=None) is None
+
+
+def test_service_timeout_and_no_result_timeout(session, request):
+    misc_options = dict(timeout=1)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=None) == 1
+
+
+def test_no_service_timeout_and_result_timeout(session, request):
+    misc_options = {}
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=1) == 1
+
+
+def test_service_timeout_lt_result_timeout(session, request):
+    misc_options = dict(timeout=10)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=11) == 11
+
+
+def test_service_timeout_gt_result_timeout(session, request):
+    misc_options = dict(timeout=11)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=10) == 11
+
+
+def test_connect_timeout_and_idle_timeout(session, request):
+    misc_options = dict(connect_timeout=1, timeout=11)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=None) == (1, 11)
+
+
+def test_connect_timeout_only(session, request):
+    misc_options = dict(connect_timeout=1)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=None) == (1, None)

From c7ee91e5e93d5b2bcd710cc7e1ae2c02f076fa84 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Fri, 24 Jul 2015 14:48:55 -0700
Subject: [PATCH 03/10] Unit tests for passing timeouts to fido_client

---
 bravado/fido_client.py                       |  2 +-
 tests/fido_client/FidoClient/request_test.py | 31 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 tests/fido_client/FidoClient/request_test.py

diff --git a/bravado/fido_client.py b/bravado/fido_client.py
index ab39fbc4..5ddd7385 100644
--- a/bravado/fido_client.py
+++ b/bravado/fido_client.py
@@ -67,7 +67,7 @@ def request(self, request_params, response_callback=None):
 
         for fetch_kwarg in ('connect_timeout', 'timeout'):
             if fetch_kwarg in request_params:
-                fetch_kwargs[fetch_kwargs] = request_params[fetch_kwarg]
+                fetch_kwargs[fetch_kwarg] = request_params[fetch_kwarg]
 
         concurrent_future = fido.fetch(url, **fetch_kwargs)
 
diff --git a/tests/fido_client/FidoClient/request_test.py b/tests/fido_client/FidoClient/request_test.py
new file mode 100644
index 00000000..e496ea01
--- /dev/null
+++ b/tests/fido_client/FidoClient/request_test.py
@@ -0,0 +1,31 @@
+from mock import patch, call
+from bravado.fido_client import FidoClient
+
+
+@patch('bravado.fido_client.fido.fetch')
+def test_timeout_passed_to_fido(fetch):
+    fido_client = FidoClient()
+    request_params = dict(url='http://foo.com/', timeout=1)
+    fido_client.request(request_params, response_callback=None)
+    assert fetch.call_args == call(
+        'http://foo.com/?', body='', headers={}, method='GET', timeout=1)
+
+
+@patch('bravado.fido_client.fido.fetch')
+def test_connect_timeout_passed_to_fido(fetch):
+    fido_client = FidoClient()
+    request_params = dict(url='http://foo.com/', connect_timeout=1)
+    fido_client.request(request_params, response_callback=None)
+    assert fetch.call_args == call(
+        'http://foo.com/?', body='', headers={}, method='GET',
+        connect_timeout=1)
+
+
+@patch('bravado.fido_client.fido.fetch')
+def test_connect_timeout_and_timeout_passed_to_fido(fetch):
+    fido_client = FidoClient()
+    request_params = dict(url='http://foo.com/', connect_timeout=1, timeout=2)
+    fido_client.request(request_params, response_callback=None)
+    assert fetch.call_args == call(
+        'http://foo.com/?', body='', headers={}, method='GET',
+        connect_timeout=1, timeout=2)

From fecd40b1790dffb7a8229724aa3024836893a3d3 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Fri, 24 Jul 2015 14:52:55 -0700
Subject: [PATCH 04/10] Update changelog; more unit tests

---
 CHANGELOG.rst                                | 4 ++++
 tests/fido_client/FidoClient/request_test.py | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index b8eabe16..fe0471a1 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,3 +1,7 @@
+2.2.0 (2015-XX-XX)
+---------------------
+- Support passing in connect_timeout and timeout via _request_options to the Fido and Requests clients
+
 2.1.0 (2015-07-20)
 ---------------------
 - Add warning for deprecated operations
diff --git a/tests/fido_client/FidoClient/request_test.py b/tests/fido_client/FidoClient/request_test.py
index e496ea01..614208da 100644
--- a/tests/fido_client/FidoClient/request_test.py
+++ b/tests/fido_client/FidoClient/request_test.py
@@ -2,6 +2,15 @@
 from bravado.fido_client import FidoClient
 
 
+@patch('bravado.fido_client.fido.fetch')
+def test_no_timeouts_passed_to_fido(fetch):
+    fido_client = FidoClient()
+    request_params = dict(url='http://foo.com/')
+    fido_client.request(request_params, response_callback=None)
+    assert fetch.call_args == call(
+        'http://foo.com/?', body='', headers={}, method='GET')
+
+
 @patch('bravado.fido_client.fido.fetch')
 def test_timeout_passed_to_fido(fetch):
     fido_client = FidoClient()

From 7fa1f8b5cfbebd6fb9980d03394e746273ea2133 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Fri, 24 Jul 2015 15:14:23 -0700
Subject: [PATCH 05/10] Fix py3 failures

---
 tests/fido_client/FidoClient/request_test.py | 68 ++++++++++++--------
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/tests/fido_client/FidoClient/request_test.py b/tests/fido_client/FidoClient/request_test.py
index 614208da..e37e22f7 100644
--- a/tests/fido_client/FidoClient/request_test.py
+++ b/tests/fido_client/FidoClient/request_test.py
@@ -1,40 +1,52 @@
-from mock import patch, call
-from bravado.fido_client import FidoClient
+from mock import patch, call, Mock
+import pytest
+import six
 
 
-@patch('bravado.fido_client.fido.fetch')
+try:
+    from bravado.fido_client import FidoClient
+except ImportError:
+    FidoClient = Mock()
+
+
+@pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
 def test_no_timeouts_passed_to_fido(fetch):
-    fido_client = FidoClient()
-    request_params = dict(url='http://foo.com/')
-    fido_client.request(request_params, response_callback=None)
-    assert fetch.call_args == call(
-        'http://foo.com/?', body='', headers={}, method='GET')
+    with patch('bravado.fido_client.fido.fetch') as fetch:
+        fido_client = FidoClient()
+        request_params = dict(url='http://foo.com/')
+        fido_client.request(request_params, response_callback=None)
+        assert fetch.call_args == call(
+            'http://foo.com/?', body='', headers={}, method='GET')
 
 
-@patch('bravado.fido_client.fido.fetch')
+@pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
 def test_timeout_passed_to_fido(fetch):
-    fido_client = FidoClient()
-    request_params = dict(url='http://foo.com/', timeout=1)
-    fido_client.request(request_params, response_callback=None)
-    assert fetch.call_args == call(
-        'http://foo.com/?', body='', headers={}, method='GET', timeout=1)
+    with patch('bravado.fido_client.fido.fetch') as fetch:
+        fido_client = FidoClient()
+        request_params = dict(url='http://foo.com/', timeout=1)
+        fido_client.request(request_params, response_callback=None)
+        assert fetch.call_args == call(
+            'http://foo.com/?', body='', headers={}, method='GET', timeout=1)
 
 
-@patch('bravado.fido_client.fido.fetch')
+@pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
 def test_connect_timeout_passed_to_fido(fetch):
-    fido_client = FidoClient()
-    request_params = dict(url='http://foo.com/', connect_timeout=1)
-    fido_client.request(request_params, response_callback=None)
-    assert fetch.call_args == call(
-        'http://foo.com/?', body='', headers={}, method='GET',
-        connect_timeout=1)
+    with patch('bravado.fido_client.fido.fetch') as fetch:
+        fido_client = FidoClient()
+        request_params = dict(url='http://foo.com/', connect_timeout=1)
+        fido_client.request(request_params, response_callback=None)
+        assert fetch.call_args == call(
+            'http://foo.com/?', body='', headers={}, method='GET',
+            connect_timeout=1)
 
 
-@patch('bravado.fido_client.fido.fetch')
+@pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
 def test_connect_timeout_and_timeout_passed_to_fido(fetch):
-    fido_client = FidoClient()
-    request_params = dict(url='http://foo.com/', connect_timeout=1, timeout=2)
-    fido_client.request(request_params, response_callback=None)
-    assert fetch.call_args == call(
-        'http://foo.com/?', body='', headers={}, method='GET',
-        connect_timeout=1, timeout=2)
+    with patch('bravado.fido_client.fido.fetch') as fetch:
+        fido_client = FidoClient()
+        request_params = dict(url='http://foo.com/', connect_timeout=1,
+                              timeout=2)
+        fido_client.request(request_params, response_callback=None)
+        assert fetch.call_args == call(
+            'http://foo.com/?', body='', headers={}, method='GET',
+            connect_timeout=1, timeout=2)

From e0791b8afd7fecd35bbc826e28df9619a1436522 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Fri, 24 Jul 2015 15:28:39 -0700
Subject: [PATCH 06/10] py3 compatibility is the pits

---
 tests/fido_client/FidoClient/request_test.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/fido_client/FidoClient/request_test.py b/tests/fido_client/FidoClient/request_test.py
index e37e22f7..9e723c55 100644
--- a/tests/fido_client/FidoClient/request_test.py
+++ b/tests/fido_client/FidoClient/request_test.py
@@ -10,7 +10,7 @@
 
 
 @pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
-def test_no_timeouts_passed_to_fido(fetch):
+def test_no_timeouts_passed_to_fido():
     with patch('bravado.fido_client.fido.fetch') as fetch:
         fido_client = FidoClient()
         request_params = dict(url='http://foo.com/')
@@ -20,7 +20,7 @@ def test_no_timeouts_passed_to_fido(fetch):
 
 
 @pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
-def test_timeout_passed_to_fido(fetch):
+def test_timeout_passed_to_fido():
     with patch('bravado.fido_client.fido.fetch') as fetch:
         fido_client = FidoClient()
         request_params = dict(url='http://foo.com/', timeout=1)
@@ -30,7 +30,7 @@ def test_timeout_passed_to_fido(fetch):
 
 
 @pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
-def test_connect_timeout_passed_to_fido(fetch):
+def test_connect_timeout_passed_to_fido():
     with patch('bravado.fido_client.fido.fetch') as fetch:
         fido_client = FidoClient()
         request_params = dict(url='http://foo.com/', connect_timeout=1)
@@ -41,7 +41,7 @@ def test_connect_timeout_passed_to_fido(fetch):
 
 
 @pytest.mark.skipif(six.PY3, reason="twisted doesnt support py3 yet")
-def test_connect_timeout_and_timeout_passed_to_fido(fetch):
+def test_connect_timeout_and_timeout_passed_to_fido():
     with patch('bravado.fido_client.fido.fetch') as fetch:
         fido_client = FidoClient()
         request_params = dict(url='http://foo.com/', connect_timeout=1,

From 162560cc3eefec70ebd442cf8776701730f303d6 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Tue, 28 Jul 2015 09:24:56 -0700
Subject: [PATCH 07/10] Use misc_options consistently

---
 bravado/requests_client.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/bravado/requests_client.py b/bravado/requests_client.py
index ae1b9e34..c679d041 100644
--- a/bravado/requests_client.py
+++ b/bravado/requests_client.py
@@ -107,16 +107,16 @@ def separate_params(request_params):
         :returns: tuple(sanitized_params, misc_options)
         """
         sanitized_params = request_params.copy()
-        request_options = {}
+        misc_options = {}
 
         if 'connect_timeout' in sanitized_params:
-            request_options['connect_timeout'] = \
+            misc_options['connect_timeout'] = \
                 sanitized_params.pop('connect_timeout')
 
         if 'timeout' in sanitized_params:
-            request_options['timeout'] = sanitized_params.pop('timeout')
+            misc_options['timeout'] = sanitized_params.pop('timeout')
 
-        return sanitized_params, request_options
+        return sanitized_params, misc_options
 
     def request(self, request_params, response_callback=None):
         """

From 984f61d951f83bb563b8994594912b1b94c10dd3 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Wed, 29 Jul 2015 11:31:54 -0700
Subject: [PATCH 08/10] Refine conflicting timeouts for the Requests client.

---
 bravado/http_future.py                        |  9 ++-
 bravado/requests_client.py                    | 62 +++++++++++--------
 .../build_timeout_test.py                     | 12 ++++
 3 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/bravado/http_future.py b/bravado/http_future.py
index 03208b18..90012aff 100644
--- a/bravado/http_future.py
+++ b/bravado/http_future.py
@@ -1,8 +1,6 @@
 # -*- coding: utf-8 -*-
 from bravado.exception import HTTPError
 
-DEFAULT_TIMEOUT_S = 5.0
-
 
 class HttpFuture(object):
     """A future which inputs HTTP params"""
@@ -20,11 +18,12 @@ def __init__(self, future, response_adapter, callback):
         self.response_adapter = response_adapter
         self.response_callback = callback
 
-    def result(self, timeout=DEFAULT_TIMEOUT_S):
+    def result(self, timeout=None):
         """Blocking call to wait for API response
 
-        :param timeout: Timeout in seconds for which client will get blocked
-        to receive the response
+        :param timeout: Number of seconds to wait for a response. Defaults to
+            None which means wait indefinitely.
+        :type timeout: float
         :return: swagger response return value when given a callback or the
             http_response otherwise.
         """
diff --git a/bravado/requests_client.py b/bravado/requests_client.py
index c679d041..fd81074e 100644
--- a/bravado/requests_client.py
+++ b/bravado/requests_client.py
@@ -197,7 +197,9 @@ def json(self, **kwargs):
 
 
 class RequestsFutureAdapter(object):
-    """A future which inputs HTTP params"""
+    """Mimics a :class:`concurrent.futures.Future` for the purposes of making
+    HTTP calls with the Requests library in a future-y sort of way.
+    """
 
     def __init__(self, session, request, misc_options):
         """Kicks API call for Requests client
@@ -220,52 +222,60 @@ def check_for_exceptions(self, response):
 
     def build_timeout(self, result_timeout):
         """
-        Build the appropriate timeout object to pass to `session.send(...)`.
+        Build the appropriate timeout object to pass to `session.send(...)`
+        based on connect_timeout, the timeout passed to the service call, and
+        the timeout passed to the result call.
 
-        :param result_timeout: timeout that was passed into `result(..)`.
+        :param result_timeout: timeout that was passed into `future.result(..)`
         :return: timeout
         :rtype: float or tuple(connect_timeout, timeout)
         """
+        # The API provides two ways to pass a timeout :( We're stuck
+        # dealing with it until we're ready to make a non-backwards
+        # compatible change.
+        #
+        #  - If both timeouts are the same, no problem
+        #  - If either has a non-None value, use the non-None value
+        #  - If both have a non-None value, use the greater of the two
         timeout = None
         has_service_timeout = 'timeout' in self.misc_options
-        has_result_timeout = result_timeout is not None
-
         service_timeout = self.misc_options.get('timeout')
-        if has_service_timeout and has_result_timeout:
-            # The API provides two ways to pass a timeout :( We're stuck
-            # dealing with it until we're ready to make a non-backwards
-            # compatible change.
+
+        if not has_service_timeout:
+            timeout = result_timeout
+        elif service_timeout == result_timeout:
             timeout = service_timeout
-            if service_timeout != result_timeout:
+        else:
+            if service_timeout is None:
+                timeout = result_timeout
+            elif result_timeout is None:
+                timeout = service_timeout
+            else:
                 timeout = max(service_timeout, result_timeout)
-                log.warn("Two different timeouts have been passed: "
-                         "_request_options['timeout'] = %s and "
-                         "future.result(timeout=%s). Using the highest "
-                         "timeout of %s."
-                         .format(service_timeout, result_timeout, timeout))
-        elif has_service_timeout:
-            timeout = service_timeout
-        elif has_result_timeout:
-            timeout = result_timeout
+            log.warn("Two different timeouts have been passed: "
+                     "_request_options['timeout'] = {0} and "
+                     "future.result(timeout={1}). Using timeout of {2}."
+                     .format(service_timeout, result_timeout, timeout))
 
+        # Requests is weird in that if you want to specify a connect_timeout
+        # and idle timeout, then the timeout is passed as a tuple
         if 'connect_timeout' in self.misc_options:
-            # Requests is weird in that if you want to specify a
-            # connect_timeout and idle timeout, then a tuple of the two is
-            # required.
-            return self.misc_options['connect_timeout'], timeout
+            timeout = self.misc_options['connect_timeout'], timeout
         return timeout
 
-    def result(self, timeout):
+    def result(self, timeout=None):
         """Blocking call to wait for API response
 
-        :param timeout: timeout in seconds to wait for response
+        :param timeout: timeout in seconds to wait for response. Defaults to
+            None to wait indefinitely.
         :type timeout: float
         :return: raw response from the server
         :rtype: dict
         """
         request = self.request
+        prepared_request = self.session.prepare_request(request)
         response = self.session.send(
-            self.session.prepare_request(request),
+            prepared_request,
             timeout=self.build_timeout(timeout))
 
         self.check_for_exceptions(response)
diff --git a/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py b/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
index 0ca04506..5517801d 100644
--- a/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
+++ b/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
@@ -45,6 +45,18 @@ def test_service_timeout_gt_result_timeout(session, request):
     assert future.build_timeout(result_timeout=10) == 11
 
 
+def test_service_timeout_None_result_timeout_not_None(session, request):
+    misc_options = dict(timeout=None)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=10) == 10
+
+
+def test_service_timeout_not_None_result_timeout_None(session, request):
+    misc_options = dict(timeout=10)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=None) == 10
+
+
 def test_connect_timeout_and_idle_timeout(session, request):
     misc_options = dict(connect_timeout=1, timeout=11)
     future = RequestsFutureAdapter(session, request, misc_options)

From 6f1483595a85b6f5f89682406437788b90bdf94e Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Wed, 29 Jul 2015 11:58:14 -0700
Subject: [PATCH 09/10] We can't have test covereage decreasing

---
 .../RequestsClient/separate_params_test.py           | 12 ++++++++++++
 .../RequestsFutureAdapter/build_timeout_test.py      | 10 ++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 tests/requests_client/RequestsClient/separate_params_test.py

diff --git a/tests/requests_client/RequestsClient/separate_params_test.py b/tests/requests_client/RequestsClient/separate_params_test.py
new file mode 100644
index 00000000..64ce78da
--- /dev/null
+++ b/tests/requests_client/RequestsClient/separate_params_test.py
@@ -0,0 +1,12 @@
+from bravado.requests_client import RequestsClient
+
+
+def test_separate_params():
+    request_params = {
+        'url': 'http://foo.com',
+        'connect_timeout': 1,
+        'timeout': 2
+    }
+    sanitized, misc = RequestsClient.separate_params(request_params)
+    assert sanitized == {'url': 'http://foo.com'}
+    assert misc == {'connect_timeout': 1, 'timeout': 2}
diff --git a/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py b/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
index 5517801d..83ab8e6c 100644
--- a/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
+++ b/tests/requests_client/RequestsFutureAdapter/build_timeout_test.py
@@ -21,13 +21,13 @@ def test_no_timeouts(session, request):
     assert future.build_timeout(result_timeout=None) is None
 
 
-def test_service_timeout_and_no_result_timeout(session, request):
+def test_service_timeout_and_result_timeout_None(session, request):
     misc_options = dict(timeout=1)
     future = RequestsFutureAdapter(session, request, misc_options)
     assert future.build_timeout(result_timeout=None) == 1
 
 
-def test_no_service_timeout_and_result_timeout(session, request):
+def test_no_service_timeout_and_result_timeout_not_None(session, request):
     misc_options = {}
     future = RequestsFutureAdapter(session, request, misc_options)
     assert future.build_timeout(result_timeout=1) == 1
@@ -57,6 +57,12 @@ def test_service_timeout_not_None_result_timeout_None(session, request):
     assert future.build_timeout(result_timeout=None) == 10
 
 
+def test_both_timeouts_the_same(session, request):
+    misc_options = dict(timeout=10)
+    future = RequestsFutureAdapter(session, request, misc_options)
+    assert future.build_timeout(result_timeout=10) == 10
+
+
 def test_connect_timeout_and_idle_timeout(session, request):
     misc_options = dict(connect_timeout=1, timeout=11)
     future = RequestsFutureAdapter(session, request, misc_options)

From c5340be77622d2c878f697400f84be778d833a94 Mon Sep 17 00:00:00 2001
From: Semir Patel <spatel@yelp.com>
Date: Mon, 3 Aug 2015 09:54:52 -0700
Subject: [PATCH 10/10] Update changelog with non-backwards compatible
 functional change to the timeout in HTTPFuture

---
 CHANGELOG.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index fe0471a1..f9009274 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,8 @@
-2.2.0 (2015-XX-XX)
+3.0.0 (2015-XX-XX)
 ---------------------
 - Support passing in connect_timeout and timeout via _request_options to the Fido and Requests clients
+- Timeout in HTTPFuture now defaults to None (wait indefinitely) instead of 5s. You should make sure
+  any calls to http_future.result(..) without a timeout are updated accordingly.
 
 2.1.0 (2015-07-20)
 ---------------------