You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Charlie Carson <cc...@twitter.com> on 2014/01/25 01:24:44 UTC

Review Request 17341: move process::http namespace into http.cpp

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

Review request for mesos, Benjamin Hindman and Jie Yu.


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


Repository: mesos-git


Description
-------

    move process::http namespace into http.cpp

    This moves the code declared in the process::http namespace to
    be in it's own file (http.cpp).  It already had it's own header.

    SEE:  https://issues.apache.org/jira/browse/MESOS-936

    Review: https://reviews.apache.org/r/17341


Diffs
-----

  3rdparty/libprocess/Makefile.am 40f01a7b3803696ccca440c8326e1d6d7c377459 
  3rdparty/libprocess/src/http.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp bc7a1c5890df7c763b0b140b47966e457e519208 

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


Testing
-------

make check


Thanks,

Charlie Carson


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/#review32780
-----------------------------------------------------------

Ship it!


Thank you for the cleanup! You'll gonna need to rebase since BenH has committed a few changes today.

- Jie Yu


On Jan. 25, 2014, 12:24 a.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2014, 12:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 40f01a7b3803696ccca440c8326e1d6d7c377459 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp bc7a1c5890df7c763b0b140b47966e457e519208 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Charlie Carson <ch...@gmail.com>.

> On Feb. 14, 2014, 12:59 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1-8
> > <https://reviews.apache.org/r/17341/diff/3/?file=484624#file484624line1>
> >
> >     Looks like we could add some includes here:
> >     
> >     #include <cstring>
> >     #include <iostream>
> >     #include <string>
> >     
> >     #include <process/future.hpp>
> >     
> >     #include <stout/lambda.hpp>
> >     #include <stout/nothing.hpp>
> >     #include <stout/option.hpp>
> >     #include <stout/os.hpp>
> >     #include <stout/try.hpp>

just added them


- Charlie


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


On Feb. 19, 2014, 8:18 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2014, 8:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 7bf2d70c59455f52b26374f78198c5c7dc0a883b 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Charlie Carson <ch...@gmail.com>.

> On Feb. 14, 2014, 12:59 a.m., Ben Mahler wrote:
> > Looks good, just wondering if we can use the new process::socket function to prevent SIGPIPE on OS X.

can we open a seperate JIRA for that ?  - I think it's good practice to keep file / directory changes separate from logic / code changes.


> On Feb. 14, 2014, 12:59 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, line 55
> > <https://reviews.apache.org/r/17341/diff/3/?file=484624#file484624line55>
> >
> >     Can this use the new socket creation function in socket.hpp? (This would prevent SIGPIPE on OS X).

created https://issues.apache.org/jira/browse/MESOS-1003 & assigned to me to do in a seperate checkin


- Charlie


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


On Feb. 13, 2014, 10:58 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2014, 10:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 2e7764a8e0badec704b8610f3f72f0bd16cc9612 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Ben Mahler <be...@gmail.com>.

> On Feb. 14, 2014, 12:59 a.m., Ben Mahler wrote:
> > Looks good, just wondering if we can use the new process::socket function to prevent SIGPIPE on OS X.
> 
> Charlie Carson wrote:
>     can we open a seperate JIRA for that ?  - I think it's good practice to keep file / directory changes separate from logic / code changes.

Thanks for the follow up, will you be adding the missing includes?


- Ben


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


On Feb. 13, 2014, 10:58 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2014, 10:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 2e7764a8e0badec704b8610f3f72f0bd16cc9612 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/#review34447
-----------------------------------------------------------

Ship it!


Looks good, just wondering if we can use the new process::socket function to prevent SIGPIPE on OS X.


3rdparty/libprocess/src/http.cpp
<https://reviews.apache.org/r/17341/#comment64561>

    Looks like we could add some includes here:
    
    #include <cstring>
    #include <iostream>
    #include <string>
    
    #include <process/future.hpp>
    
    #include <stout/lambda.hpp>
    #include <stout/nothing.hpp>
    #include <stout/option.hpp>
    #include <stout/os.hpp>
    #include <stout/try.hpp>



3rdparty/libprocess/src/http.cpp
<https://reviews.apache.org/r/17341/#comment64560>

    Can this use the new socket creation function in socket.hpp? (This would prevent SIGPIPE on OS X).


- Ben Mahler


On Feb. 13, 2014, 10:58 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2014, 10:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 2e7764a8e0badec704b8610f3f72f0bd16cc9612 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/#review34927
-----------------------------------------------------------

Ship it!


In addition to the 'socket' follow up can you check if we can kill any includes from process.cpp?

- Benjamin Hindman


On Feb. 19, 2014, 8:18 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2014, 8:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 7bf2d70c59455f52b26374f78198c5c7dc0a883b 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/#review34926
-----------------------------------------------------------

Ship it!


In addition to the 'socket' follow up can you check if we can kill any includes from process.cpp?

- Benjamin Hindman


On Feb. 19, 2014, 8:18 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2014, 8:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 7bf2d70c59455f52b26374f78198c5c7dc0a883b 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Charlie Carson <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/
-----------------------------------------------------------

(Updated Feb. 19, 2014, 8:18 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
-------

rebase from master & added missing #include's to new http.cpp file


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


Repository: mesos-git


Description
-------

    move process::http namespace into http.cpp

    This moves the code declared in the process::http namespace to
    be in it's own file (http.cpp).  It already had it's own header.

    SEE:  https://issues.apache.org/jira/browse/MESOS-936

    Review: https://reviews.apache.org/r/17341


Diffs (updated)
-----

  3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
  3rdparty/libprocess/src/http.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 7bf2d70c59455f52b26374f78198c5c7dc0a883b 

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


Testing
-------

make check


Thanks,

Charlie Carson


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/#review34445
-----------------------------------------------------------

Ship it!


Ship It!

- Jie Yu


On Feb. 13, 2014, 10:58 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17341/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2014, 10:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-936
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-936
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     move process::http namespace into http.cpp
> 
>     This moves the code declared in the process::http namespace to
>     be in it's own file (http.cpp).  It already had it's own header.
> 
>     SEE:  https://issues.apache.org/jira/browse/MESOS-936
> 
>     Review: https://reviews.apache.org/r/17341
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
>   3rdparty/libprocess/src/http.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 2e7764a8e0badec704b8610f3f72f0bd16cc9612 
> 
> Diff: https://reviews.apache.org/r/17341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Charlie Carson <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/
-----------------------------------------------------------

(Updated Feb. 13, 2014, 10:58 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


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


Repository: mesos-git


Description
-------

    move process::http namespace into http.cpp

    This moves the code declared in the process::http namespace to
    be in it's own file (http.cpp).  It already had it's own header.

    SEE:  https://issues.apache.org/jira/browse/MESOS-936

    Review: https://reviews.apache.org/r/17341


Diffs (updated)
-----

  3rdparty/libprocess/Makefile.am bcb78a45788fe93c520a787c2b614d49c354e633 
  3rdparty/libprocess/src/http.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 2e7764a8e0badec704b8610f3f72f0bd16cc9612 

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


Testing
-------

make check


Thanks,

Charlie Carson


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Charlie Carson <cc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/
-----------------------------------------------------------

(Updated Jan. 26, 2014, 2:17 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
-------

pull in changes made to master


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


Repository: mesos-git


Description
-------

    move process::http namespace into http.cpp

    This moves the code declared in the process::http namespace to
    be in it's own file (http.cpp).  It already had it's own header.

    SEE:  https://issues.apache.org/jira/browse/MESOS-936

    Review: https://reviews.apache.org/r/17341


Diffs (updated)
-----

  3rdparty/libprocess/Makefile.am 40f01a7b3803696ccca440c8326e1d6d7c377459 
  3rdparty/libprocess/src/http.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp c28fc35e2fc2e5416691f52f2209e894d72e1c20 

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


Testing
-------

make check


Thanks,

Charlie Carson


Re: Review Request 17341: move process::http namespace into http.cpp

Posted by Charlie Carson <cc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17341/
-----------------------------------------------------------

(Updated Jan. 25, 2014, 12:24 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


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


Repository: mesos-git


Description
-------

    move process::http namespace into http.cpp

    This moves the code declared in the process::http namespace to
    be in it's own file (http.cpp).  It already had it's own header.

    SEE:  https://issues.apache.org/jira/browse/MESOS-936

    Review: https://reviews.apache.org/r/17341


Diffs
-----

  3rdparty/libprocess/Makefile.am 40f01a7b3803696ccca440c8326e1d6d7c377459 
  3rdparty/libprocess/src/http.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp bc7a1c5890df7c763b0b140b47966e457e519208 

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


Testing
-------

make check


Thanks,

Charlie Carson