-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37403/#review95296
-----------------------------------------------------------

Ship it!


Some minor comments and noticed that we were not setting the response content 
type! I'll get these cleaned up and commit


src/tests/http_api_tests.cpp (line 41)
<https://reviews.apache.org/r/37403/#comment150168>

    Why the move?



src/tests/http_api_tests.cpp (line 563)
<https://reviews.apache.org/r/37403/#comment150172>

    How about 'NotAcceptable'?



src/tests/http_api_tests.cpp (line 572)
<https://reviews.apache.org/r/37403/#comment150169>

    No need for the std:: prefix here?



src/tests/http_api_tests.cpp (line 575)
<https://reviews.apache.org/r/37403/#comment150186>

    It's pretty implicit but http::streaming::post is already being passed the 
content type and will overwrite it, so this doesn't do anything.



src/tests/http_api_tests.cpp (line 576)
<https://reviews.apache.org/r/37403/#comment150171>

    How about we write a valid format? e.g. text/html
    
    Note that these are supposed to be formatted as type/subtype



src/tests/http_api_tests.cpp (line 606)
<https://reviews.apache.org/r/37403/#comment150187>

    This get overwritten to the parameterized content type when we pass 
'contentType' into http::streaming::post. Otherwise this test would fail 
because we encode as either PROTOBUF or JSON depending on the test parameter.



src/tests/http_api_tests.cpp (line 660)
<https://reviews.apache.org/r/37403/#comment150188>

    In all of these tests, how about we also validate the content type coming 
back in the response? Seems an important thing to test for 'Accept' working 
correctly.
    
    In particular, these don't test the behavior that JSON is the default?
    
    By adding this, I found a bug :) We aren't setting the content type of the 
response!


- Ben Mahler


On Aug. 12, 2015, 11:12 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37403/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2015, 11:12 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2497
>     https://issues.apache.org/jira/browse/MESOS-2497
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Using AcceptMediaType Request method to validate request 'Accept' HTTP header 
> in Scheduler HTTP API
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 579c009 
>   src/tests/http_api_tests.cpp aef3c4b 
> 
> Diff: https://reviews.apache.org/r/37403/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Reply via email to