mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <bbann...@apache.org>
Subject Re: Review Request 71833: Created unix domain socket on agent startup.
Date Fri, 10 Jan 2020 13:33:34 GMT

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


Fix it, then Ship it!





src/common/domain_sockets.hpp
Lines 31 (patched)
<https://reviews.apache.org/r/71833/#comment307325>

    Can you get rid of the `using` decl and use the FQN instead?



src/common/domain_sockets.hpp
Lines 39 (patched)
<https://reviews.apache.org/r/71833/#comment307326>

    Can you get rid of this variable and instead directly return `*socket`?



src/common/domain_sockets.hpp
Lines 80-81 (patched)
<https://reviews.apache.org/r/71833/#comment307327>

    Are we not returning an `Error` here on purpose? Without context that would seem preferable.
If we do indeed want to allow this to fail we should add a comment explaining the reasoning.



src/slave/main.cpp
Lines 59-61 (original), 59-62 (patched)
<https://reviews.apache.org/r/71833/#comment307328>

    Let's keep these sorted.



src/slave/main.cpp
Lines 111 (patched)
<https://reviews.apache.org/r/71833/#comment307329>

    This looks unused.



src/slave/main.cpp
Line 352 (original), 358 (patched)
<https://reviews.apache.org/r/71833/#comment307330>

    Reuse a generic constant instead of magic `108`.



src/slave/main.cpp
Lines 629 (patched)
<https://reviews.apache.org/r/71833/#comment307331>

    Can you init this with `None`?



src/slave/main.cpp
Lines 633 (patched)
<https://reviews.apache.org/r/71833/#comment307332>

    You can use `CHECK_SOME` here.



src/tests/cluster.cpp
Lines 124 (patched)
<https://reviews.apache.org/r/71833/#comment307333>

    This looks unused (and sorted incorrectly).



src/tests/cluster.cpp
Lines 614 (patched)
<https://reviews.apache.org/r/71833/#comment307335>

    Init with `None`.



src/tests/cluster.cpp
Lines 618 (patched)
<https://reviews.apache.org/r/71833/#comment307334>

    `CHECK_SOME`


- Benjamin Bannier


On Jan. 10, 2020, 2:46 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71833/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2020, 2:46 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-10036
>     https://issues.apache.org/jira/browse/MESOS-10036
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added agent code to check whether domain socket support is
> enabled, and if so to create or open the socket at
> MESOS_DOMAIN_SOCKET_LOCATION.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 47ad1bd27e155f0193aafa956df0bd43baafd348 
>   src/common/domain_sockets.hpp PRE-CREATION 
>   src/local/local.cpp 8ff43618f06463b4ba86b64b25e5de692e406448 
>   src/slave/main.cpp fd58637cd680291e6794bcdb0655603bb97744c7 
>   src/slave/slave.hpp 77b5bc0082c6bb73fbd48a2ebe812629921645cb 
>   src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 
>   src/tests/cluster.cpp f7bc882a644ec65710ada3d15507e1d4c5ba06f7 
>   src/tests/mock_slave.cpp 71be957884ea88258ef37e60649e3947e89b12d0 
> 
> 
> Diff: https://reviews.apache.org/r/71833/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message