From e82f28ea4596380948c3d98df0f466e09f53ddb9 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Sat, 18 Nov 2017 17:02:12 +0100 Subject: [PATCH 01/13] bravado depends on latest bravado-core due to MSGPACK features --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index b48fa02..f9ba314 100755 --- a/setup.py +++ b/setup.py @@ -31,7 +31,7 @@ "Programming Language :: Python :: 3.6", ], install_requires=[ - "bravado-core >= 4.2.2", + "bravado-core >= 4.11.0", "python-dateutil", "pyyaml", "requests >= 2", From 43501b2bd6cf9ff971f151f9b45c9f6235358282 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Sat, 18 Nov 2017 17:03:14 +0100 Subject: [PATCH 02/13] Avoid calling bravado-core for handling of HTTP respones related objects --- bravado/client.py | 24 ++-------------- bravado/config_defaults.py | 18 ++++++++++++ bravado/http_future.py | 57 ++++++++++++++++++++++++++++++++------ 3 files changed, 70 insertions(+), 29 deletions(-) create mode 100644 bravado/config_defaults.py diff --git a/bravado/client.py b/bravado/client.py index c57c244..782a219 100644 --- a/bravado/client.py +++ b/bravado/client.py @@ -53,6 +53,8 @@ from six import iteritems from six import itervalues +from bravado.config_defaults import CONFIG_DEFAULTS +from bravado.config_defaults import REQUEST_OPTIONS_DEFAULTS from bravado.docstring_property import docstring_property from bravado.requests_client import RequestsClient from bravado.swagger_model import Loader @@ -61,25 +63,6 @@ log = logging.getLogger(__name__) -CONFIG_DEFAULTS = { - # See the constructor of :class:`bravado.http_future.HttpFuture` for an - # in depth explanation of what this means. - 'also_return_response': False, -} - -REQUEST_OPTIONS_DEFAULTS = { - # List of callbacks that are executed after the incoming response has been - # validated and the swagger_result has been unmarshalled. - # - # The callback should expect two arguments: - # param : incoming_response - # type : subclass of class:`bravado_core.response.IncomingResponse` - # param : operation - # type : class:`bravado_core.operation.Operation` - 'response_callbacks': [], -} - - class SwaggerClient(object): """A client for accessing a Swagger-documented RESTful service. @@ -91,8 +74,7 @@ def __init__(self, swagger_spec, also_return_response=False): self.swagger_spec = swagger_spec @classmethod - def from_url(cls, spec_url, http_client=None, request_headers=None, - config=None): + def from_url(cls, spec_url, http_client=None, request_headers=None, config=None): """Build a :class:`SwaggerClient` from a url to the Swagger specification for a RESTful API. diff --git a/bravado/config_defaults.py b/bravado/config_defaults.py new file mode 100644 index 0000000..97ec3aa --- /dev/null +++ b/bravado/config_defaults.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +CONFIG_DEFAULTS = { + # See the constructor of :class:`bravado.http_future.HttpFuture` for an + # in depth explanation of what this means. + 'also_return_response': False, +} + +REQUEST_OPTIONS_DEFAULTS = { + # List of callbacks that are executed after the incoming response has been + # validated and the swagger_result has been unmarshalled. + # + # The callback should expect two arguments: + # param : incoming_response + # type : subclass of class:`bravado_core.response.IncomingResponse` + # param : operation + # type : class:`bravado_core.operation.Operation` + 'response_callbacks': [], +} diff --git a/bravado/http_future.py b/bravado/http_future.py index 6a74869..bdb17a4 100644 --- a/bravado/http_future.py +++ b/bravado/http_future.py @@ -2,10 +2,16 @@ import sys from functools import wraps -import bravado_core import six +import umsgpack +from bravado_core.content_type import APP_JSON +from bravado_core.content_type import APP_MSGPACK from bravado_core.exception import MatchingResponseNotFound +from bravado_core.response import get_response_spec +from bravado_core.unmarshal import unmarshal_schema_object +from bravado_core.validate import validate_schema_object +from bravado.config_defaults import REQUEST_OPTIONS_DEFAULTS from bravado.exception import BravadoTimeoutError from bravado.exception import make_http_exception @@ -92,7 +98,7 @@ def __init__(self, future, response_adapter, operation=None, self.future = future self.response_adapter = response_adapter self.operation = operation - self.response_callbacks = response_callbacks or [] + self.response_callbacks = response_callbacks or REQUEST_OPTIONS_DEFAULTS['response_callbacks'] self.also_return_response = also_return_response @reraise_errors @@ -130,12 +136,10 @@ def unmarshal_response(incoming_response, operation, response_callbacks=None): This hands the response over to bravado_core for validation and unmarshalling and then runs any response callbacks. On success, the swagger_result is available as ``incoming_response.swagger_result``. - :type incoming_response: :class:`bravado_core.response.IncomingResponse` :type operation: :class:`bravado_core.operation.Operation` :type response_callbacks: list of callable. See bravado_core.client.REQUEST_OPTIONS_DEFAULTS. - :raises: HTTPError - On 5XX status code, the HTTPError has minimal information. - On non-2XX status code with no matching response, the HTTPError @@ -147,10 +151,10 @@ def unmarshal_response(incoming_response, operation, response_callbacks=None): try: raise_on_unexpected(incoming_response) - incoming_response.swagger_result = \ - bravado_core.response.unmarshal_response( - incoming_response, - operation) + incoming_response.swagger_result = unmarshal_response_inner( + response=incoming_response, + op=operation, + ) except MatchingResponseNotFound as e: exception = make_http_exception( response=incoming_response, @@ -168,6 +172,43 @@ def unmarshal_response(incoming_response, operation, response_callbacks=None): raise_on_expected(incoming_response) +def unmarshal_response_inner(response, op): + """ + Unmarshal incoming http response into a value based on the + response specification. + :type response: :class:`bravado_core.response.IncomingResponse` + :type op: :class:`bravado_core.operation.Operation` + :returns: value where type(value) matches response_spec['schema']['type'] + if it exists, None otherwise. + """ + deref = op.swagger_spec.deref + response_spec = get_response_spec(status_code=response.status_code, op=op) + + if 'schema' not in response_spec: + return None + + content_type = response.headers.get('content-type', '').lower() + + if content_type.startswith(APP_JSON) or content_type.startswith(APP_MSGPACK): + content_spec = deref(response_spec['schema']) + if content_type.startswith(APP_JSON): + content_value = response.json() + else: + content_value = umsgpack.unpackb(response.raw_bytes) + + if op.swagger_spec.config.get('validate_responses', False): + validate_schema_object(op.swagger_spec, content_spec, content_value) + + return unmarshal_schema_object( + swagger_spec=op.swagger_spec, + schema_object_spec=content_spec, + value=content_value, + ) + + # TODO: Non-json response contents + return response.text + + def raise_on_unexpected(http_response): """Raise an HTTPError if the response is 5XX. From 154aa94bc285c9559d45ebc59e9748646f2ab4ad Mon Sep 17 00:00:00 2001 From: Alston Lin Date: Mon, 4 Dec 2017 15:06:22 -0800 Subject: [PATCH 03/13] Add a _use_msgpack option --- bravado/client.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bravado/client.py b/bravado/client.py index c57c244..9471242 100644 --- a/bravado/client.py +++ b/bravado/client.py @@ -275,7 +275,7 @@ def __call__(self, **op_kwargs): also_return_response=also_return_response) -def construct_request(operation, request_options, **op_kwargs): +def construct_request(operation, request_options, _use_msgpack=False, **op_kwargs): """Construct the outgoing request dict. :type operation: :class:`bravado_core.operation.Operation` @@ -293,6 +293,9 @@ def construct_request(operation, request_options, **op_kwargs): 'params': {}, # filled in downstream 'headers': request_options.get('headers', {}), } + # Adds Accept header to request for msgpack response if specified + if _use_msgpack: + request['headers']['Accept'] = 'application/msgpack' # Copy over optional request options for request_option in ('connect_timeout', 'timeout'): From a8b7e21628cae9ee19d88ddd876a03db40dfa259 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Wed, 6 Dec 2017 10:45:18 +0100 Subject: [PATCH 04/13] Fix tests --- tests/http_future/unmarshall_response_test.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tests/http_future/unmarshall_response_test.py b/tests/http_future/unmarshall_response_test.py index 12fc5fa..e19599b 100644 --- a/tests/http_future/unmarshall_response_test.py +++ b/tests/http_future/unmarshall_response_test.py @@ -10,6 +10,12 @@ from bravado.http_future import unmarshal_response +@pytest.fixture +def mock_unmarshal_response_inner(): + with patch('bravado.http_future.unmarshal_response_inner') as m: + yield m + + def test_5XX(): incoming_response = Mock(spec=IncomingResponse, status_code=500) operation = Mock(spec=Operation) @@ -18,8 +24,8 @@ def test_5XX(): assert excinfo.value.response.status_code == 500 -@patch('bravado_core.response.unmarshal_response', return_value=99) -def test_2XX(_1): +def test_2XX(mock_unmarshal_response_inner): + mock_unmarshal_response_inner.return_value = 99 incoming_response = Mock(spec=IncomingResponse) incoming_response.status_code = 200 operation = Mock(spec=Operation) @@ -27,9 +33,8 @@ def test_2XX(_1): assert incoming_response.swagger_result == 99 -@patch('bravado_core.response.unmarshal_response', - side_effect=MatchingResponseNotFound('boo')) -def test_2XX_matching_response_not_found_in_spec(_1): +def test_2XX_matching_response_not_found_in_spec(mock_unmarshal_response_inner): + mock_unmarshal_response_inner.side_effect = MatchingResponseNotFound('boo') incoming_response = Mock(spec=IncomingResponse, status_code=200) operation = Mock(spec=Operation) with pytest.raises(HTTPError) as excinfo: @@ -38,9 +43,8 @@ def test_2XX_matching_response_not_found_in_spec(_1): assert excinfo.value.message == 'boo' -@patch('bravado_core.response.unmarshal_response', - side_effect=MatchingResponseNotFound) -def test_4XX_matching_response_not_found_in_spec(_1): +def test_4XX_matching_response_not_found_in_spec(mock_unmarshal_response_inner): + mock_unmarshal_response_inner.side_effect = MatchingResponseNotFound incoming_response = Mock(spec=IncomingResponse, status_code=404) operation = Mock(spec=Operation) with pytest.raises(HTTPError) as excinfo: @@ -48,9 +52,8 @@ def test_4XX_matching_response_not_found_in_spec(_1): assert excinfo.value.response.status_code == 404 -@patch('bravado_core.response.unmarshal_response', - return_value={'msg': 'Not found'}) -def test_4XX(_1): +def test_4XX(mock_unmarshal_response_inner): + mock_unmarshal_response_inner.return_value = {'msg': 'Not found'} incoming_response = Mock(spec=IncomingResponse, status_code=404) operation = Mock(spec=Operation) with pytest.raises(HTTPError) as excinfo: @@ -59,8 +62,8 @@ def test_4XX(_1): assert excinfo.value.swagger_result == {'msg': 'Not found'} -@patch('bravado_core.response.unmarshal_response', return_value=99) -def test_response_callbacks_executed_on_happy_path(_1): +def test_response_callbacks_executed_on_happy_path(mock_unmarshal_response_inner): + mock_unmarshal_response_inner.return_value = 99 incoming_response = Mock(spec=IncomingResponse) incoming_response.status_code = 200 operation = Mock(spec=Operation) @@ -71,8 +74,8 @@ def test_response_callbacks_executed_on_happy_path(_1): assert callback.call_count == 1 -@patch('bravado_core.response.unmarshal_response', return_value=99) -def test_response_callbacks_executed_on_failure(_1): +def test_response_callbacks_executed_on_failure(mock_unmarshal_response_inner): + mock_unmarshal_response_inner.return_value = 99 incoming_response = Mock(spec=IncomingResponse, status_code=404) operation = Mock(spec=Operation) callback = Mock() From f67a834b27a1a53b00cf4aef49748751331acdda Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Wed, 6 Dec 2017 11:02:56 +0100 Subject: [PATCH 05/13] Add unmarshal_response_inner tests --- .../unmarshall_response_inner_test.py | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/http_future/unmarshall_response_inner_test.py diff --git a/tests/http_future/unmarshall_response_inner_test.py b/tests/http_future/unmarshall_response_inner_test.py new file mode 100644 index 0000000..230989c --- /dev/null +++ b/tests/http_future/unmarshall_response_inner_test.py @@ -0,0 +1,119 @@ +# -*- coding: utf-8 -*- +import mock +import msgpack +import pytest +from bravado_core.content_type import APP_JSON +from bravado_core.content_type import APP_MSGPACK +from bravado_core.response import IncomingResponse +from bravado_core.spec import Spec + +from bravado.http_future import unmarshal_response_inner + + +@pytest.fixture +def empty_swagger_spec(): + return Spec(spec_dict={}) + + +@pytest.fixture +def response_spec(): + return { + 'description': "Day of the week", + 'schema': { + 'type': 'string', + } + } + + +@pytest.fixture +def mock_get_response_spec(): + with mock.patch('bravado.http_future.get_response_spec') as m: + yield m + + +@pytest.fixture +def mock_validate_schema_object(): + with mock.patch('bravado.http_future.validate_schema_object') as m: + yield m + + +def test_no_content(mock_get_response_spec, empty_swagger_spec): + response_spec = { + 'description': "I don't have a 'schema' key so I return nothing", + } + response = mock.Mock(spec=IncomingResponse, status_code=200) + + mock_get_response_spec.return_value = response_spec + op = mock.Mock(swagger_spec=empty_swagger_spec) + result = unmarshal_response_inner(response, op) + assert result is None + + +def test_json_content(mock_get_response_spec, empty_swagger_spec, response_spec): + response = mock.Mock( + spec=IncomingResponse, + status_code=200, + headers={'content-type': APP_JSON}, + json=mock.Mock(return_value='Monday'), + ) + + mock_get_response_spec.return_value = response_spec + op = mock.Mock(swagger_spec=empty_swagger_spec) + assert 'Monday' == unmarshal_response_inner(response, op) + + +def test_msgpack_content(mock_get_response_spec, empty_swagger_spec, response_spec): + message = 'Monday' + response = mock.Mock( + spec=IncomingResponse, + status_code=200, + headers={'content-type': APP_MSGPACK}, + raw_bytes=msgpack.dumps(message), + ) + + mock_get_response_spec.return_value = response_spec + op = mock.Mock(swagger_spec=empty_swagger_spec) + assert message == unmarshal_response_inner(response, op) + + +def test_text_content(mock_get_response_spec, empty_swagger_spec, response_spec): + response = mock.Mock( + spec=IncomingResponse, + status_code=200, + headers={'content-type': 'text/plain'}, + text='Monday', + ) + + mock_get_response_spec.return_value = response_spec + op = mock.Mock(swagger_spec=empty_swagger_spec) + assert 'Monday' == unmarshal_response_inner(response, op) + + +def test_skips_validation(mock_validate_schema_object, mock_get_response_spec, empty_swagger_spec, response_spec): + empty_swagger_spec.config['validate_responses'] = False + response = mock.Mock( + spec=IncomingResponse, + status_code=200, + headers={'content-type': APP_JSON}, + json=mock.Mock(return_value='Monday'), + ) + + mock_get_response_spec.return_value = response_spec + op = mock.Mock(swagger_spec=empty_swagger_spec) + unmarshal_response_inner(response, op) + assert mock_validate_schema_object.call_count == 0 + + +def test_performs_validation(mock_validate_schema_object, mock_get_response_spec, empty_swagger_spec, response_spec): + empty_swagger_spec.config['validate_responses'] = True + response = mock.Mock( + spec=IncomingResponse, + status_code=200, + headers={'content-type': APP_JSON}, + json=mock.Mock(return_value='Monday'), + ) + + mock_get_response_spec.return_value = response_spec + op = mock.Mock(swagger_spec=empty_swagger_spec) + unmarshal_response_inner(response, op) + assert mock_validate_schema_object.call_count == 1 From 3b4c06042dc0145c57be401a2d8e95ffa3c04339 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Wed, 6 Dec 2017 11:23:24 +0100 Subject: [PATCH 06/13] Add msgpack-python dependency --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index f9ba314..e06b2ad 100755 --- a/setup.py +++ b/setup.py @@ -32,6 +32,7 @@ ], install_requires=[ "bravado-core >= 4.11.0", + "msgpack-python", "python-dateutil", "pyyaml", "requests >= 2", From b02d462d69dc21ce36c31cc66aa4953761b9bc34 Mon Sep 17 00:00:00 2001 From: Alston Lin Date: Wed, 6 Dec 2017 14:16:11 -0800 Subject: [PATCH 07/13] use_msgpack is now in _request_options and added integration tests for it --- bravado/client.py | 4 ++-- tests/integration/conftest.py | 18 ++++++++++++------ tests/integration/requests_client_test.py | 18 +++++++++++++++--- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/bravado/client.py b/bravado/client.py index 9471242..eaae08d 100644 --- a/bravado/client.py +++ b/bravado/client.py @@ -275,7 +275,7 @@ def __call__(self, **op_kwargs): also_return_response=also_return_response) -def construct_request(operation, request_options, _use_msgpack=False, **op_kwargs): +def construct_request(operation, request_options, **op_kwargs): """Construct the outgoing request dict. :type operation: :class:`bravado_core.operation.Operation` @@ -294,7 +294,7 @@ def construct_request(operation, request_options, _use_msgpack=False, **op_kwarg 'headers': request_options.get('headers', {}), } # Adds Accept header to request for msgpack response if specified - if _use_msgpack: + if request_options.get('use_msgpack', False): request['headers']['Accept'] = 'application/msgpack' # Copy over optional request options diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index ae29859..5c6d258 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -43,9 +43,12 @@ }, }, }, - '/msgpack': { + '/json_or_msgpack': { 'get': { - 'produces': ['application/msgpack'], + 'produces': [ + 'application/msgpack', + 'application/json' + ], 'responses': { '200': { 'description': 'HTTP/200', @@ -69,10 +72,13 @@ def api_json(): return API_RESPONSE -@bottle.route('/msgpack') -def api_msgpack(): - bottle.response.content_type = APP_MSGPACK - return umsgpack.packb(API_RESPONSE) +@bottle.route('/json_or_msgpack') +def api_json_or_msgpack(): + if bottle.request.headers.get('accept') == APP_MSGPACK: + bottle.response.content_type = APP_MSGPACK + return umsgpack.packb(API_RESPONSE) + else: + return API_RESPONSE @bottle.route('/1') diff --git a/tests/integration/requests_client_test.py b/tests/integration/requests_client_test.py index b2d8d8c..89bc845 100644 --- a/tests/integration/requests_client_test.py +++ b/tests/integration/requests_client_test.py @@ -60,8 +60,17 @@ def test_swagger_client_json_response(self, swagger_client): assert marshaled_response == API_RESPONSE assert raw_response.raw_bytes == json.dumps(API_RESPONSE).encode('utf-8') - def test_swagger_client_msgpack_response(self, swagger_client): - marshaled_response, raw_response = swagger_client.msgpack.get_msgpack().result(timeout=1) + def test_swagger_client_msgpack_response_without_flag(self, swagger_client): + marshaled_response, raw_response = swagger_client.json_or_msgpack.get_json_or_msgpack().result(timeout=1) + assert marshaled_response == API_RESPONSE + assert raw_response.raw_bytes == json.dumps(API_RESPONSE).encode('utf-8') + + def test_swagger_client_msgpack_response_with_flag(self, swagger_client): + marshaled_response, raw_response = swagger_client.json_or_msgpack.get_json_or_msgpack( + _request_options={ + 'use_msgpack': True, + }, + ).result(timeout=1) assert marshaled_response == API_RESPONSE assert raw_response.raw_bytes == umsgpack.packb(API_RESPONSE) @@ -115,8 +124,11 @@ def test_boolean_header(self, threaded_http_server): def test_msgpack_support(self, threaded_http_server): response = self.http_client.request({ 'method': 'GET', - 'url': '{server_address}/msgpack'.format(server_address=threaded_http_server), + 'url': '{server_address}/json_or_msgpack'.format(server_address=threaded_http_server), 'params': {}, + 'headers': { + 'Accept': APP_MSGPACK, + }, }).result(timeout=1) assert response.headers['Content-Type'] == APP_MSGPACK From 95f28d3ae6fcb894a5be371c936b31a5088a86a2 Mon Sep 17 00:00:00 2001 From: Stephan Jaensch Date: Thu, 7 Dec 2017 13:07:05 +0100 Subject: [PATCH 08/13] We should use ReadTimeout for the requests client --- bravado/requests_client.py | 3 ++- tests/integration/requests_client_test.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/bravado/requests_client.py b/bravado/requests_client.py index d1d4efd..0b29da3 100644 --- a/bravado/requests_client.py +++ b/bravado/requests_client.py @@ -3,6 +3,7 @@ import requests import requests.auth +import requests.exceptions import six from bravado_core.response import IncomingResponse from six import iteritems @@ -215,7 +216,7 @@ class RequestsFutureAdapter(FutureAdapter): HTTP calls with the Requests library in a future-y sort of way. """ - timeout_errors = [requests.Timeout] + timeout_errors = [requests.exceptions.ReadTimeout] def __init__(self, session, request, misc_options): """Kicks API call for Requests client diff --git a/tests/integration/requests_client_test.py b/tests/integration/requests_client_test.py index b2d8d8c..d459095 100644 --- a/tests/integration/requests_client_test.py +++ b/tests/integration/requests_client_test.py @@ -4,6 +4,7 @@ import mock import pytest import requests +import requests.exceptions import umsgpack from bravado_core.content_type import APP_MSGPACK @@ -147,6 +148,17 @@ def test_timeout_errors_are_catchable_with_original_exception_types(self, thread 'params': {}, }).result(timeout=0.01) + def test_timeout_errors_are_catchable_as_requests_timeout(self, threaded_http_server): + if not self.http_client_type == RequestsClient: + pytest.skip('{} is not using RequestsClient'.format(self.http_future_adapter_type)) + + with pytest.raises(requests.exceptions.Timeout): + self.http_client.request({ + 'method': 'GET', + 'url': '{server_address}/sleep?sec=0.1'.format(server_address=threaded_http_server), + 'params': {}, + }).result(timeout=0.01) + class FakeRequestsFutureAdapter(RequestsFutureAdapter): timeout_errors = [] From 403d2539eb9e3c52d1d4210b942edec9852128fe Mon Sep 17 00:00:00 2001 From: Stephan Jaensch Date: Thu, 7 Dec 2017 13:26:53 +0100 Subject: [PATCH 09/13] Release version 9.2.1 --- CHANGELOG.rst | 4 ++++ bravado/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2db87a1..8cfce03 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,10 @@ Changelog ========= +9.2.1 (2017-12-07) +------------------ +- The timeout exception for the requests client should inherit from ``requests.exceptions.ReadTimeout`` instead of ``requests.exceptions.Timeout`` - PR #337 + 9.2.0 (2017-11-10) ------------------ - Support msgpack as wire format for response data - PR #323, 328, 330, 331 diff --git a/bravado/__init__.py b/bravado/__init__.py index 773a491..216069a 100644 --- a/bravado/__init__.py +++ b/bravado/__init__.py @@ -1,2 +1,2 @@ # -*- coding: utf-8 -*- -version = '9.2.0' +version = '9.2.1' From bda393be33a96c45f89d6fb5c7030618d126245b Mon Sep 17 00:00:00 2001 From: Alston Lin Date: Thu, 7 Dec 2017 11:14:00 -0800 Subject: [PATCH 10/13] Added more tests and documentation on the use_msgpack request option --- docs/source/configuration.rst | 2 ++ tests/client/construct_request_test.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index c796566..bc90250 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -82,4 +82,6 @@ Config key Type Default Description | - ``operation`` of type ``bravado_core.operation.Operation`` *timeout* float N/A | TCP idle timeout in seconds. This is passed along to the | http_client when making a service call. +*use_msgpack* boolean False | If a msgpack serialization is desired for the response. This + | will add a Accept: application/msgpack header to the request. ========================= =============== ========= =============================================================== diff --git a/tests/client/construct_request_test.py b/tests/client/construct_request_test.py index 0050fad..5b2abe1 100644 --- a/tests/client/construct_request_test.py +++ b/tests/client/construct_request_test.py @@ -83,3 +83,25 @@ def test_with_not_string_headers( assert request['headers'][header_name] == str(header_value) unmarshalled_request = unmarshal_request(request_object, operation) assert unmarshalled_request[header_name] == header_value + + +def test_use_msgpack( + minimal_swagger_spec, + getPetById_spec, +): + op = CallableOperation( + Operation.from_spec( + minimal_swagger_spec, + '/pet/{petId}', + 'get', + getPetById_spec + ) + ) + request = construct_request( + op, + request_options={ + 'use_msgpack': True, + }, + petId=1, + ) + assert request['headers']['Accept'] == 'application/msgpack' From f475f770d469479c092b27afa105a1c5f5d4a720 Mon Sep 17 00:00:00 2001 From: Jesse Myers Date: Mon, 18 Dec 2017 10:57:36 -0800 Subject: [PATCH 11/13] Using umsgpack breaks because it is not a dependency --- bravado/http_future.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bravado/http_future.py b/bravado/http_future.py index bdb17a4..49c4b9a 100644 --- a/bravado/http_future.py +++ b/bravado/http_future.py @@ -3,7 +3,7 @@ from functools import wraps import six -import umsgpack +from msgpack import unpackb from bravado_core.content_type import APP_JSON from bravado_core.content_type import APP_MSGPACK from bravado_core.exception import MatchingResponseNotFound @@ -194,7 +194,7 @@ def unmarshal_response_inner(response, op): if content_type.startswith(APP_JSON): content_value = response.json() else: - content_value = umsgpack.unpackb(response.raw_bytes) + content_value = unpackb(response.raw_bytes) if op.swagger_spec.config.get('validate_responses', False): validate_schema_object(op.swagger_spec, content_spec, content_value) From 8a29025c7ba1aa81176c23044c293c819f056827 Mon Sep 17 00:00:00 2001 From: Jesse Myers Date: Mon, 18 Dec 2017 11:45:33 -0800 Subject: [PATCH 12/13] Remove umsgpack from tests and properly unpack with UTF-8 --- bravado/http_future.py | 2 +- tests/integration/conftest.py | 4 ++-- tests/integration/requests_client_test.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bravado/http_future.py b/bravado/http_future.py index 49c4b9a..76898ec 100644 --- a/bravado/http_future.py +++ b/bravado/http_future.py @@ -194,7 +194,7 @@ def unmarshal_response_inner(response, op): if content_type.startswith(APP_JSON): content_value = response.json() else: - content_value = unpackb(response.raw_bytes) + content_value = unpackb(response.raw_bytes, encoding='utf-8') if op.swagger_spec.config.get('validate_responses', False): validate_schema_object(op.swagger_spec, content_spec, content_value) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5c6d258..57cf440 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -5,7 +5,7 @@ import bottle import ephemeral_port_reserve import pytest -import umsgpack +from msgpack import packb from bravado_core.content_type import APP_JSON from bravado_core.content_type import APP_MSGPACK from six.moves import urllib @@ -76,7 +76,7 @@ def api_json(): def api_json_or_msgpack(): if bottle.request.headers.get('accept') == APP_MSGPACK: bottle.response.content_type = APP_MSGPACK - return umsgpack.packb(API_RESPONSE) + return packb(API_RESPONSE) else: return API_RESPONSE diff --git a/tests/integration/requests_client_test.py b/tests/integration/requests_client_test.py index 04943ed..d2d4475 100644 --- a/tests/integration/requests_client_test.py +++ b/tests/integration/requests_client_test.py @@ -5,7 +5,7 @@ import pytest import requests import requests.exceptions -import umsgpack +from msgpack import packb, unpackb from bravado_core.content_type import APP_MSGPACK from bravado.client import SwaggerClient @@ -73,7 +73,7 @@ def test_swagger_client_msgpack_response_with_flag(self, swagger_client): }, ).result(timeout=1) assert marshaled_response == API_RESPONSE - assert raw_response.raw_bytes == umsgpack.packb(API_RESPONSE) + assert raw_response.raw_bytes == packb(API_RESPONSE) def test_multiple_requests(self, threaded_http_server): request_one_params = { @@ -133,7 +133,7 @@ def test_msgpack_support(self, threaded_http_server): }).result(timeout=1) assert response.headers['Content-Type'] == APP_MSGPACK - assert umsgpack.unpackb(response.raw_bytes) == API_RESPONSE + assert unpackb(response.raw_bytes, encoding='utf-8') == API_RESPONSE def test_timeout_errors_are_thrown_as_BravadoTimeoutError(self, threaded_http_server): timeout_errors = getattr(self.http_future_adapter_type, 'timeout_errors', []) From e6a160adcbd08f2a0d949e27113df06df67f0661 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Mon, 18 Dec 2017 20:55:49 +0100 Subject: [PATCH 13/13] Release version 9.2.2 --- CHANGELOG.rst | 4 ++++ bravado/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8cfce03..c90997b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,10 @@ Changelog ========= +9.2.2 (2017-12-19) +------------------ +- Fix msgpack import issue - PR #341. Thanks Jesse Myers for your contribution! + 9.2.1 (2017-12-07) ------------------ - The timeout exception for the requests client should inherit from ``requests.exceptions.ReadTimeout`` instead of ``requests.exceptions.Timeout`` - PR #337 diff --git a/bravado/__init__.py b/bravado/__init__.py index 216069a..6cde369 100644 --- a/bravado/__init__.py +++ b/bravado/__init__.py @@ -1,2 +1,2 @@ # -*- coding: utf-8 -*- -version = '9.2.1' +version = '9.2.2'