-----------------------------------------------------------
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
> 
>

Reply via email to