You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2014/10/29 06:18:20 UTC
Re: Review Request 27054: Libprocess Socket: introduce connect()
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/
-----------------------------------------------------------
(Updated Oct. 29, 2014, 5:18 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
rebase on 27023
Bugs: MESOS-1330
https://issues.apache.org/jira/browse/MESOS-1330
Repository: mesos-git
Description
-------
Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
Diffs (updated)
-----
3rdparty/libprocess/include/process/socket.hpp 6683881
3rdparty/libprocess/src/process.cpp 85fb995
Diff: https://reviews.apache.org/r/27054/diff/
Testing
-------
make check
Thanks,
Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Oct. 29, 2014, 4:57 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> >
> > Reviews applied: [27350, 27351, 27350]
> >
> > Failed command: ./support/apply-review.sh -n -r 27350
> >
> > Error:
> > 2014-10-29 23:57:20 URL:https://reviews.apache.org/r/27350/diff/raw/ [897/897] -> "27350.patch" [1]
> > error: patch failed: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4:37
> > error: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4: patch does not apply
> > Failed to apply patch
Do you need to rebase?
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review59083
-----------------------------------------------------------
On Oct. 29, 2014, 3:42 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27054/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 3:42 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Bugs: MESOS-1330
> https://issues.apache.org/jira/browse/MESOS-1330
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/socket.hpp 6683881
> 3rdparty/libprocess/src/process.cpp 85fb995
>
> Diff: https://reviews.apache.org/r/27054/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review59083
-----------------------------------------------------------
Bad patch!
Reviews applied: [27350, 27351, 27350]
Failed command: ./support/apply-review.sh -n -r 27350
Error:
2014-10-29 23:57:20 URL:https://reviews.apache.org/r/27350/diff/raw/ [897/897] -> "27350.patch" [1]
error: patch failed: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4:37
error: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Oct. 29, 2014, 10:42 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27054/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 10:42 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Bugs: MESOS-1330
> https://issues.apache.org/jira/browse/MESOS-1330
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/socket.hpp 6683881
> 3rdparty/libprocess/src/process.cpp 85fb995
>
> Diff: https://reviews.apache.org/r/27054/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/
-----------------------------------------------------------
(Updated Oct. 29, 2014, 10:42 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Depends on enable_shared_from_this configure check for libprocess.
Bugs: MESOS-1330
https://issues.apache.org/jira/browse/MESOS-1330
Repository: mesos-git
Description
-------
Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
Diffs
-----
3rdparty/libprocess/include/process/socket.hpp 6683881
3rdparty/libprocess/src/process.cpp 85fb995
Diff: https://reviews.apache.org/r/27054/diff/
Testing
-------
make check
Thanks,
Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review59063
-----------------------------------------------------------
Patch looks great!
Reviews applied: [27350, 27023, 27054]
All tests passed.
- Mesos ReviewBot
On Oct. 29, 2014, 9:47 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27054/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 9:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Bugs: MESOS-1330
> https://issues.apache.org/jira/browse/MESOS-1330
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/socket.hpp 6683881
> 3rdparty/libprocess/src/process.cpp 85fb995
>
> Diff: https://reviews.apache.org/r/27054/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/
-----------------------------------------------------------
(Updated Oct. 29, 2014, 9:47 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Depends on mutex configure check for libprocess.
Bugs: MESOS-1330
https://issues.apache.org/jira/browse/MESOS-1330
Repository: mesos-git
Description
-------
Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
Diffs
-----
3rdparty/libprocess/include/process/socket.hpp 6683881
3rdparty/libprocess/src/process.cpp 85fb995
Diff: https://reviews.apache.org/r/27054/diff/
Testing
-------
make check
Thanks,
Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review58907
-----------------------------------------------------------
3rdparty/libprocess/include/process/socket.hpp
<https://reviews.apache.org/r/27054/#comment100075>
We name our enumerations like constants, and we name our constants in capital letters, like macros. See: http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Enumerator_Names (but note that we don't use the 'k' prefix style for naming, which was only recently "accepted" for enumerations in the Google Style Guide).
Check out other examples in the code base too.
3rdparty/libprocess/include/process/socket.hpp
<https://reviews.apache.org/r/27054/#comment100076>
{ on newline please.
3rdparty/libprocess/include/process/socket.hpp
<https://reviews.apache.org/r/27054/#comment100078>
This should be virtual, no?
3rdparty/libprocess/include/process/socket.hpp
<https://reviews.apache.org/r/27054/#comment100147>
We've put friend declarations at the top of the private section because, if you're not a friend, you have no reason to read below that line! ;-)
In all seriousness, the rationale here is exactly the same reasoning for why we order public, protected, then private: the class declaration acts as documentation, and most people care about the public members of the class first.
3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/27054/#comment100150>
I'd like us to call this struct Connect instead of AsyncConnect since this will not be the first time that we have to create a struct for holding data across continuations and it would be great to set a precedent for naming continuation data. That is, this is a fundamental pattern that we'll replicate. So in the future, we can have an internal::Write which we know maps to a 'write' continuation, etc. Sound good?
3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/27054/#comment100151>
Let's stay consistent with how we label our continuations: s/async_receiving_connect/_connect/.
- Benjamin Hindman
On Oct. 29, 2014, 5:18 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27054/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 5:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Bugs: MESOS-1330
> https://issues.apache.org/jira/browse/MESOS-1330
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/socket.hpp 6683881
> 3rdparty/libprocess/src/process.cpp 85fb995
>
> Diff: https://reviews.apache.org/r/27054/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review58966
-----------------------------------------------------------
Patch looks great!
Reviews applied: [27023, 27054]
All tests passed.
- Mesos ReviewBot
On Oct. 29, 2014, 5:18 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27054/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 5:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Bugs: MESOS-1330
> https://issues.apache.org/jira/browse/MESOS-1330
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/socket.hpp 6683881
> 3rdparty/libprocess/src/process.cpp 85fb995
>
> Diff: https://reviews.apache.org/r/27054/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 27054: Libprocess Socket: introduce connect()
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review59042
-----------------------------------------------------------
3rdparty/libprocess/include/process/socket.hpp
<https://reviews.apache.org/r/27054/#comment100324>
not without a check in configure, you don't :)
3rdparty/libprocess/include/process/socket.hpp
<https://reviews.apache.org/r/27054/#comment100325>
needs a check in configure.
- Dominic Hamon
On Oct. 28, 2014, 10:18 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27054/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 10:18 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Bugs: MESOS-1330
> https://issues.apache.org/jira/browse/MESOS-1330
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce the connect(const Node& node) call to Socket. Use it in link() and send().
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/socket.hpp 6683881
> 3rdparty/libprocess/src/process.cpp 85fb995
>
> Diff: https://reviews.apache.org/r/27054/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joris Van Remoortere
>
>