You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/03/21 00:51:24 UTC
Review Request 32347: Added a StreamingResponseDecoder.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32347/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Jie Yu.
Bugs: MESOS-2438
https://issues.apache.org/jira/browse/MESOS-2438
Repository: mesos
Description
-------
Provides a response decoder that returns 'PIPE' responses once the response headers are received, but before the body data is received. Callers are expected to read the body from the Pipe::Reader in the response.
This is needed to receive streaming responses on the client-side.
Diffs
-----
3rdparty/libprocess/src/decoder.hpp 45f3d7fff2c33ce9c37a8ecbdcbf2c0f02ee76e8
3rdparty/libprocess/src/tests/decoder_tests.cpp d65f5cf25eaecd5994404ccdf07f32d703a5955b
Diff: https://reviews.apache.org/r/32347/diff/
Testing
-------
Added a test.
Thanks,
Ben Mahler
Re: Review Request 32347: Added a StreamingResponseDecoder.
Posted by Ben Mahler <be...@gmail.com>.
> On March 27, 2015, 12:20 a.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp, lines 177-178
> > <https://reviews.apache.org/r/32347/diff/1/?file=901908#file901908line177>
> >
> > Do you want to move this up right after
> > ```
> > decoder.decode(body.data(), body.length());
> > ```
> > ?
Sounds better, thanks!
> On March 27, 2015, 12:20 a.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/decoder.hpp, line 480
> > <https://reviews.apache.org/r/32347/diff/1/?file=901907#file901907line480>
> >
> > Looks like we won't be using the old ResponseDecoder anymore after https://reviews.apache.org/r/32351.
> >
> > Are you planing to kill the old response decoder? If yes, the name "Streaming" here seems to be uncessary once the old response decoder is killed.
> >
> > Are you going to s/StreamingResponseDecoder/ResponseDecoder/ after r32351 is committed?
Yeah, I'll remove the old decoder in a follow up change, thanks!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32347/#review77967
-----------------------------------------------------------
On March 20, 2015, 11:51 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32347/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 11:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jie Yu.
>
>
> Bugs: MESOS-2438
> https://issues.apache.org/jira/browse/MESOS-2438
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Provides a response decoder that returns 'PIPE' responses once the response headers are received, but before the body data is received. Callers are expected to read the body from the Pipe::Reader in the response.
>
> This is needed to receive streaming responses on the client-side.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/decoder.hpp 45f3d7fff2c33ce9c37a8ecbdcbf2c0f02ee76e8
> 3rdparty/libprocess/src/tests/decoder_tests.cpp d65f5cf25eaecd5994404ccdf07f32d703a5955b
>
> Diff: https://reviews.apache.org/r/32347/diff/
>
>
> Testing
> -------
>
> Added a test.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 32347: Added a StreamingResponseDecoder.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32347/#review77967
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/src/decoder.hpp
<https://reviews.apache.org/r/32347/#comment126336>
Looks like we won't be using the old ResponseDecoder anymore after https://reviews.apache.org/r/32351.
Are you planing to kill the old response decoder? If yes, the name "Streaming" here seems to be uncessary once the old response decoder is killed.
Are you going to s/StreamingResponseDecoder/ResponseDecoder/ after r32351 is committed?
3rdparty/libprocess/src/decoder.hpp
<https://reviews.apache.org/r/32347/#comment126346>
```
CHECK_NOTNULL(decoder->response);
```
?
3rdparty/libprocess/src/tests/decoder_tests.cpp
<https://reviews.apache.org/r/32347/#comment126372>
Do you want to move this up right after
```
decoder.decode(body.data(), body.length());
```
?
- Jie Yu
On March 20, 2015, 11:51 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32347/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 11:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jie Yu.
>
>
> Bugs: MESOS-2438
> https://issues.apache.org/jira/browse/MESOS-2438
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Provides a response decoder that returns 'PIPE' responses once the response headers are received, but before the body data is received. Callers are expected to read the body from the Pipe::Reader in the response.
>
> This is needed to receive streaming responses on the client-side.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/decoder.hpp 45f3d7fff2c33ce9c37a8ecbdcbf2c0f02ee76e8
> 3rdparty/libprocess/src/tests/decoder_tests.cpp d65f5cf25eaecd5994404ccdf07f32d703a5955b
>
> Diff: https://reviews.apache.org/r/32347/diff/
>
>
> Testing
> -------
>
> Added a test.
>
>
> Thanks,
>
> Ben Mahler
>
>