----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28451/#review63056 -----------------------------------------------------------
src/test/python/apache/aurora/common/test_transport.py <https://reviews.apache.org/r/28451/#comment105225> This test doesn't really do what it says. It's just testing that we call raise_for_status, but ideally what we'd be testing is that unsuccessful status codes (500 in this case) on responses result in exceptions. I.e. the test is dependent on the implementation rather than simply testing the expected behavior. A more accurate test would be actually generate a mock 500 response rather than mocking raise_for_status to raise an error. I'm not sure how arduous that would be, if it's an excessive amount of work then I could live with just renaming the test to something that more accurately describes what it's doing. - Joshua Cohen On Nov. 25, 2014, 9:22 p.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28451/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2014, 9:22 p.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-949 > https://issues.apache.org/jira/browse/AURORA-949 > > > Repository: aurora > > > Description > ------- > > 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. > > > Diffs > ----- > > src/main/python/apache/aurora/common/transport.py > 6f7c355d725b5e537cc4ae471170eaa8431da326 > src/test/python/apache/aurora/common/test_transport.py > c722eae2d04dec90e9c772f49c578184a2bdf76c > > Diff: https://reviews.apache.org/r/28451/diff/ > > > Testing > ------- > > ./pants src/test/python/apache/aurora/common:test_transport -v > > > Thanks, > > Zameer Manji > >
