You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ke...@apache.org on 2014/12/02 22:31:48 UTC
incubator-aurora git commit: Modify TRequestsTransport to raise an
exception 4xx or 5xx responses.
Repository: incubator-aurora
Updated Branches:
refs/heads/master c1b078d56 -> dc666e7e4
Modify TRequestsTransport to raise an exception 4xx or 5xx responses.
This changes TRequestsTransport to raise an exception on 4xx or
5xx responses. Previously the TRequestsTransport would ignore those
responses and treat the response as valid. The response would not be
valid JSON and cause thrift decoding errors later on.
Testing Done:
./pants src/test/python/apache/aurora/common:test_transport -v
Bugs closed: AURORA-949
Reviewed at https://reviews.apache.org/r/28451/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/dc666e7e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/dc666e7e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/dc666e7e
Branch: refs/heads/master
Commit: dc666e7e4663dc696a818b8961ccb147edd8e70d
Parents: c1b078d
Author: Zameer Manji <zm...@twopensource.com>
Authored: Tue Dec 2 13:31:27 2014 -0800
Committer: Kevin Sweeney <ke...@apache.org>
Committed: Tue Dec 2 13:31:44 2014 -0800
----------------------------------------------------------------------
.../python/apache/aurora/common/transport.py | 1 +
.../apache/aurora/common/test_transport.py | 40 ++++++++++++++------
2 files changed, 29 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dc666e7e/src/main/python/apache/aurora/common/transport.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/common/transport.py b/src/main/python/apache/aurora/common/transport.py
index fbdb1ad..76e079a 100644
--- a/src/main/python/apache/aurora/common/transport.py
+++ b/src/main/python/apache/aurora/common/transport.py
@@ -102,6 +102,7 @@ class TRequestsTransport(TTransportBase):
data=data,
timeout=self.__timeout,
auth=self.__auth)
+ response.raise_for_status()
except request_exceptions.Timeout:
raise TTransportException(
type=TTransportException.TIMED_OUT,
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dc666e7e/src/test/python/apache/aurora/common/test_transport.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/common/test_transport.py b/src/test/python/apache/aurora/common/test_transport.py
index dd066bf..2045f64 100644
--- a/src/test/python/apache/aurora/common/test_transport.py
+++ b/src/test/python/apache/aurora/common/test_transport.py
@@ -15,6 +15,7 @@
import logging
from threading import Thread
+import pytest
import requests
from mock import create_autospec, Mock
from requests import exceptions as request_exceptions
@@ -67,16 +68,36 @@ def test_request_transport_timeout():
protocol = TJSONProtocol.TJSONProtocol(transport)
client = ReadOnlyScheduler.Client(protocol)
- try:
+ with pytest.raises(TTransport.TTransportException) as execinfo:
+ client.getRoleSummary()
+
+ assert execinfo.value.message == 'Timed out talking to http://localhost:12345'
+
+ transport.close()
+
+
+def test_raise_for_status_causes_exception():
+ response = create_autospec(spec=requests.Response, instance=True)
+ response.raise_for_status.side_effect = requests.exceptions.HTTPError()
+
+ session = create_autospec(spec=requests.Session, instance=True)
+ session.headers = {}
+ session.post.return_value = response
+
+ transport = TRequestsTransport('http://localhost:12345', session_factory=lambda: session)
+ protocol = TJSONProtocol.TJSONProtocol(transport)
+ client = ReadOnlyScheduler.Client(protocol)
+
+ with pytest.raises(TTransport.TTransportException) as excinfo:
client.getRoleSummary()
- assert False, 'getRoleSummary should not succeed'
- except TTransport.TTransportException as e:
- assert e.message == 'Timed out talking to http://localhost:12345'
- except Exception as e:
- assert False, 'Only expected TTransportException, got %s' % e
+
+ assert excinfo.value.type == TTransport.TTransportException.UNKNOWN
+ assert excinfo.value.message.startswith('Unknown error talking to http://localhost:12345')
transport.close()
+ response.raise_for_status.assert_called_once_with()
+
def test_request_any_other_exception():
session = create_autospec(spec=requests.Session, instance=True)
@@ -86,13 +107,8 @@ def test_request_any_other_exception():
protocol = TJSONProtocol.TJSONProtocol(transport)
client = ReadOnlyScheduler.Client(protocol)
- try:
+ with pytest.raises(TTransport.TTransportException):
client.getRoleSummary()
- assert False, 'getRoleSummary should not succeed'
- except TTransport.TTransportException:
- pass
- except Exception as e:
- assert False, 'Only expected TTransportException, got %s' % e
transport.close()