You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Nikita Vetoshkin <ni...@gmail.com> on 2014/08/22 18:10:58 UTC

Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

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

Review request for mesos.


Bugs: MESOS-1625
    https://issues.apache.org/jira/browse/MESOS-1625


Repository: mesos-git


Description
-------

As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.


Diffs
-----

  3rdparty/libprocess/src/encoder.hpp 9c5aa8134c101214d740bf231559ce5b5d51764c 

Diff: https://reviews.apache.org/r/24984/diff/


Testing
-------

`make check` and manual testing with golang language native binding


Thanks,

Nikita Vetoshkin

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Nikita Vetoshkin <ni...@gmail.com>.

> On Sept. 5, 2014, 1:45 a.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [24984]
> > 
> > All tests passed.

Bot thinks patch is okay. Can someone take a look?


- Nikita


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


On Sept. 4, 2014, 9:28 a.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 9:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Dominic Hamon <dh...@twopensource.com>.

> On Sept. 4, 2014, 6:45 p.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [24984]
> > 
> > All tests passed.
> 
> Nikita Vetoshkin wrote:
>     Bot thinks patch is okay. Can someone take a look?

there's no shepherd assigned so this might fall through the cracks. I think this looks fine but I don't know the reason why we used chunked in the first place. [~benjaminhindman] should probably respond.


- Dominic


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


On Sept. 4, 2014, 2:28 a.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 2:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24984/#review52400
-----------------------------------------------------------


Patch looks great!

Reviews applied: [24984]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2014, 9:28 a.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 9:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Nikita Vetoshkin <ni...@gmail.com>.

> On Sept. 4, 2014, 5:06 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/encoder.hpp, line 124
> > <https://reviews.apache.org/r/24984/diff/2/?file=676526#file676526line124>
> >
> >     i'm not even sure how this was working given that Transfer-Encoding is an HTTP/1.1 feature.

I guess by virtue of all modern http clients which are very tolerable :)


- Nikita


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


On Sept. 4, 2014, 9:28 a.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 9:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24984/#review52317
-----------------------------------------------------------



3rdparty/libprocess/src/encoder.hpp
<https://reviews.apache.org/r/24984/#comment91081>

    i'm not even sure how this was working given that Transfer-Encoding is an HTTP/1.1 feature.


- Dominic Hamon


On Sept. 4, 2014, 2:28 a.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 2:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Nikita Vetoshkin <ni...@gmail.com>.

> On Sept. 5, 2014, 5:01 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/encoder.hpp, line 124
> > <https://reviews.apache.org/r/24984/diff/2/?file=676526#file676526line124>
> >
> >     there are a few other places in the code that work with Transfer-Encoding. check http.hpp and process.cpp. also http_tests.cpp checks for this header.

I took a look at all files you mentioned. In all cases "chunked" is used in "PIPE" mode: declared in http.hpp, implemented in process.cpp, tested in http_tests.cpp.


- Nikita


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


On Sept. 5, 2014, 5 p.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2014, 5 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24984/#review52470
-----------------------------------------------------------



3rdparty/libprocess/src/encoder.hpp
<https://reviews.apache.org/r/24984/#comment91264>

    there are a few other places in the code that work with Transfer-Encoding. check http.hpp and process.cpp. also http_tests.cpp checks for this header.


- Dominic Hamon


On Sept. 5, 2014, 10 a.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24984/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2014, 10 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
> 
> 
> Bugs: MESOS-1625
>     https://issues.apache.org/jira/browse/MESOS-1625
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9c5aa81 
> 
> Diff: https://reviews.apache.org/r/24984/diff/
> 
> 
> Testing
> -------
> 
> `make check` and manual testing with golang language native binding
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Nikita Vetoshkin <ni...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24984/
-----------------------------------------------------------

(Updated Sept. 5, 2014, 10 a.m.)


Review request for mesos, Benjamin Hindman and Dominic Hamon.


Changes
-------

added reviewers


Bugs: MESOS-1625
    https://issues.apache.org/jira/browse/MESOS-1625


Repository: mesos-git


Description
-------

As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.


Diffs
-----

  3rdparty/libprocess/src/encoder.hpp 9c5aa81 

Diff: https://reviews.apache.org/r/24984/diff/


Testing
-------

`make check` and manual testing with golang language native binding


Thanks,

Nikita Vetoshkin

Re: Review Request 24984: Libprocess: Use Content-Length instead of Encoding: Chunked

Posted by Nikita Vetoshkin <ni...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24984/
-----------------------------------------------------------

(Updated Sept. 4, 2014, 9:28 a.m.)


Review request for mesos.


Changes
-------

No changes - just trigger build


Bugs: MESOS-1625
    https://issues.apache.org/jira/browse/MESOS-1625


Repository: mesos-git


Description
-------

As mentioned in MESOS-1625 current HTTP Chunked Encoding implementation can break clients (e.g. Tornado). As I see no need in Chunked-Encoding because we know content size beforehand - I propose to use Content-Length.


Diffs (updated)
-----

  3rdparty/libprocess/src/encoder.hpp 9c5aa81 

Diff: https://reviews.apache.org/r/24984/diff/


Testing
-------

`make check` and manual testing with golang language native binding


Thanks,

Nikita Vetoshkin