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

Ship it!


Let me make sure I understand the backwards compatibility matrix here, assuming 
this lands in 0.19.0:

(1) 0.18.0 libprocess -> 0.19.0 libprocess: OK (we don't send responses in 
presence of "libprocess" User-Agent)
(2) 0.19.0 libprocess -> 0.18.0 libprocess: OK (old libprocess never replies)

(3) custom libprocess -> 0.18.0 libprocess: safe, custom libprocess gets no 202 
replies
(4) 0.18.0 libprocess -> custom libprocess:
  (a) SocketManager::link will use recv_data, which expects to decode requests 
but fails and closes socket upon receiving a response?
  (b) SocketManager::send sockets will not be reading, TCP buffers will fill up 
and the custom libprocess send() calls will fail eventually?


3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/20276/#comment75429>

    Maybe a comment that this ensures the libev watcher will consider the 
socket ready for reading? Is that what's required here?



3rdparty/libprocess/src/tests/process_tests.cpp
<https://reviews.apache.org/r/20276/#comment75430>

    How does this test still pass? Seems like you're no longer sending a 
response when User-Agent contains "/libprocess", something I'm missing?
    
    Would it be better to split up this test into two cases:
    
    sending from libprocess -> no response
    sending from fake libprocess -> 202


- Ben Mahler


On April 30, 2014, 3:51 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20276/
> -----------------------------------------------------------
> 
> (Updated April 30, 2014, 3:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian 
> Wickman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c 
>   3rdparty/libprocess/src/process.cpp 
> 26c16cf58c31102dc61b5844b3e4d75e5bc2764e 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 745c3ada5f55722aed4adb4d0b1fcb16e4cb8e9b 
> 
> Diff: https://reviews.apache.org/r/20276/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Reply via email to