Merge pull request Yelp#218 from lucagiovagnoli/fido-3.0.0

Fido 3.0.0 - EventualResult instead of concurrent futures - Fix requests unicode bug

CHANGELOG.rst

@@ -1,5 +1,11 @@
 Changelog
 =========
+8.2.0 (2016-04-29)
+------------------
+- Bravado compliant to Fido 3.0.0 
+- Dropped use of concurrent futures in favor of crochet EventualResult
+- Workaround for bypassing a unicode bug in python `requests` < 2.8.1
+
 8.1.2 (2016-04-18)
 ------------------
 - Don't unnecessarily constrain the version of twisted when not using python 2.6

bravado/__init__.py

@@ -1 +1 @@
-version = '8.1.2'
+version = '8.2.0'

bravado/fido_client.py

@@ -10,6 +10,7 @@
 import fido
 from bravado_core.response import IncomingResponse
 from bravado.http_client import HttpClient
+from bravado.http_future import FutureAdapter
 from bravado.http_future import HttpFuture
 
 log = logging.getLogger(__name__)
@@ -70,9 +71,9 @@ def request(self, request_params, operation=None, response_callbacks=None,
 
         request_for_twisted = self.prepare_request_for_twisted(request_params)
 
-        concurrent_future = fido.fetch(**request_for_twisted)
+        future_adapter = FidoFutureAdapter(fido.fetch(**request_for_twisted))
 
-        return HttpFuture(concurrent_future,
+        return HttpFuture(future_adapter,
                           FidoResponseAdapter,
                           operation,
                           response_callbacks,
@@ -114,7 +115,10 @@ def prepare_request_for_twisted(request_params):
         prepared_request.headers.pop('Content-Length', None)
 
         request_for_twisted = {
-            'method': prepared_request.method or 'GET',
+            # converting to string for `requests` method is necessary when
+            # using requests < 2.8.1 due to a bug while handling unicode values
+            # See changelog 2.8.1 at https://pypi.python.org/pypi/requests
+            'method': str(prepared_request.method or 'GET'),
             'body': prepared_request.body,
             'headers': prepared_request.headers,
             'url': prepared_request.url,
@@ -125,3 +129,17 @@ def prepare_request_for_twisted(request_params):
                 request_for_twisted[fetch_kwarg] = request_params[fetch_kwarg]
 
         return request_for_twisted
+
+
+class FidoFutureAdapter(FutureAdapter):
+    """
+    This is just a wrapper for an EventualResult object from crochet.
+    It implements the 'result' method which is needed by our HttpFuture to
+    retrieve results.
+    """
+
+    def __init__(self, eventual_result):
+        self._eventual_result = eventual_result
+
+    def result(self, timeout=None):
+        return self._eventual_result.wait(timeout=timeout)

bravado/http_future.py

@@ -8,12 +8,33 @@
 from bravado.exception import HTTPError
 
 
+class FutureAdapter(object):
+    """
+    Mimics a :class:`concurrent.futures.Future` regardless of which client is
+    performing the request, whether it is synchronous or actually asynchornous.
+
+    This adapter must be implemented by all bravado clients such as FidoClient
+    or RequestsClient to wrap the object returned by their 'request' method.
+
+    """
+
+    def result(self, timeout=None):
+        """
+        Must implement a result method which blocks on result retrieval.
+
+        :param timeout: maximum time to wait on result retrieval. Defaults to
+            None which means blocking undefinitely.
+        """
+        raise NotImplementedError(
+            "FutureAdapter must implement 'result' method"
+        )
+
+
 class HttpFuture(object):
-    """Wrapper for a :class:`concurrent.futures.Future` that returns an HTTP
-    response.
+    """Wrapper for a :class:`FutureAdapter` that returns an HTTP response.
 
-    :param concurrent_future: The concurrent concurrent_future to wrap.
-    :type concurrent_future: :class: `concurrent.futures.Future`
+    :param future: The future object to wrap.
+    :type future: :class: `FutureAdapter`
     :param response_adapter: Adapter type which exposes the innards of the HTTP
         response in a non-http client specific way.
     :type response_adapter: type that is a subclass of
@@ -29,9 +50,9 @@ class HttpFuture(object):
         http response code, etc.
         Defaults to False for backwards compatibility.
     """
-    def __init__(self, concurrent_future, response_adapter, operation=None,
+    def __init__(self, future, response_adapter, operation=None,
                  response_callbacks=None, also_return_response=False):
-        self.concurrent_future = concurrent_future
+        self.future = future
         self.response_adapter = response_adapter
         self.operation = operation
         self.response_callbacks = response_callbacks or []
@@ -46,7 +67,7 @@ def result(self, timeout=None):
         :return: Depends on the value of also_return_response sent in
             to the constructor.
         """
-        inner_response = self.concurrent_future.result(timeout=timeout)
+        inner_response = self.future.result(timeout=timeout)
         incoming_response = self.response_adapter(inner_response)
 
         if self.operation is not None:

bravado/requests_client.py

@@ -7,6 +7,7 @@
 from six.moves.urllib import parse as urlparse
 
 from bravado.http_client import HttpClient
+from bravado.http_future import FutureAdapter
 from bravado.http_future import HttpFuture
 
 
@@ -193,7 +194,7 @@ def json(self, **kwargs):
         return self._delegate.json(**kwargs)
 
 
-class RequestsFutureAdapter(object):
+class RequestsFutureAdapter(FutureAdapter):
     """Mimics a :class:`concurrent.futures.Future` for the purposes of making
     HTTP calls with the Requests library in a future-y sort of way.
     """

tests/http_future/HttpFuture/result_test.py

@@ -1,6 +1,6 @@
 from bravado_core.operation import Operation
 from bravado_core.response import IncomingResponse
-from concurrent.futures import Future
+from bravado.http_future import FutureAdapter
 from mock import patch, Mock
 import pytest
 
@@ -11,7 +11,7 @@ def test_200_get_swagger_spec():
     response_adapter_instance = Mock(spec=IncomingResponse, status_code=200)
     response_adapter_type = Mock(return_value=response_adapter_instance)
     http_future = HttpFuture(
-        concurrent_future=Mock(spec=Future),
+        future=Mock(spec=FutureAdapter),
         response_adapter=response_adapter_type)
 
     assert response_adapter_instance == http_future.result()
@@ -23,7 +23,7 @@ def test_500_get_swagger_spec():
 
     with pytest.raises(HTTPError) as excinfo:
         HttpFuture(
-            concurrent_future=Mock(spec=Future),
+            future=Mock(spec=FutureAdapter),
             response_adapter=response_adapter_type).result()
 
     assert excinfo.value.response.status_code == 500
@@ -39,7 +39,7 @@ def test_200_service_call(_):
     response_adapter_type = Mock(return_value=response_adapter_instance)
 
     http_future = HttpFuture(
-        concurrent_future=Mock(spec=Future),
+        future=Mock(spec=FutureAdapter),
         response_adapter=response_adapter_type,
         operation=Mock(spec=Operation))
 
@@ -56,7 +56,7 @@ def test_400_service_call(mock_unmarshal_response):
     response_adapter_type = Mock(return_value=response_adapter_instance)
 
     http_future = HttpFuture(
-        concurrent_future=Mock(spec=Future),
+        future=Mock(spec=FutureAdapter),
         response_adapter=response_adapter_type,
         operation=Mock(spec=Operation))
 
@@ -76,7 +76,7 @@ def test_also_return_response_true(_):
     response_adapter_type = Mock(return_value=response_adapter_instance)
 
     http_future = HttpFuture(
-        concurrent_future=Mock(spec=Future),
+        future=Mock(spec=FutureAdapter),
         response_adapter=response_adapter_type,
         operation=Mock(spec=Operation),
         also_return_response=True)

tests/integration/fido_client_test.py

@@ -76,10 +76,10 @@ def test_multiple_requests_against_fido_client(self):
             'params': {},
         }
 
-        eventual_one = self.fido_client.request(request_one_params)
-        eventual_two = self.fido_client.request(request_two_params)
-        resp_one = eventual_one.result(timeout=1)
-        resp_two = eventual_two.result(timeout=1)
+        http_future_1 = self.fido_client.request(request_one_params)
+        http_future_2 = self.fido_client.request(request_two_params)
+        resp_one = http_future_1.result(timeout=1)
+        resp_two = http_future_2.result(timeout=1)
 
         assert resp_one.text == ROUTE_1_RESPONSE
         assert resp_two.text == ROUTE_2_RESPONSE
@@ -93,7 +93,7 @@ def test_post_request(self):
             'data': {"number": 3},
         }
 
-        eventual = self.fido_client.request(request_args)
-        resp = eventual.result(timeout=1)
+        http_future = self.fido_client.request(request_args)
+        resp = http_future.result(timeout=1)
 
         assert resp.text == '6'