mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 53704: Added a level of indirection for logger through an IO Switchboard.
Date Tue, 22 Nov 2016 22:10:47 GMT

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


Fix it, then Ship it!





src/CMakeLists.txt (line 329)
<https://reviews.apache.org/r/53704/#comment226610>

    I'd suggest to use "mesos/io/switchboard.cpp" and "mesos/io/switchboard_main.cpp" in case
we have other io related classes or functions to move to the same folder.
    
    In the future, maybe we want to move logger to 'io/' subfolder as well.



src/slave/containerizer/mesos/containerizer.hpp (line 293)
<https://reviews.apache.org/r/53704/#comment226841>

    const?



src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp (lines 51 - 58)
<https://reviews.apache.org/r/53704/#comment226838>

    I'd suggest we initialize those in a constructor and move the impl to the cpp file.



src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp (line 61)
<https://reviews.apache.org/r/53704/#comment226837>

    Should we return `Try<Owned<IOSwitchboard>>` here?



src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp (line 28)
<https://reviews.apache.org/r/53704/#comment226842>

    kill this line



src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp (line 68)
<https://reviews.apache.org/r/53704/#comment226840>

    insert a new line above.


- Jie Yu


On Nov. 20, 2016, 8:33 p.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53704/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2016, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6467
>     https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The purpose of this component is to feed stdin to a container from an
> external source, as well as redirect the stdin/stdout of a container
> to multiple targets.
> 
> In this commit, we simply add the IOSwitchboard as a component that
> interposes on the fds set up to communicate between the logger and a
> container.
> 
> In the future, we will expand this component to (optionaly) launch a
> sidecar HTTP server process which will be responsible for handling
> 'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
> of a container to redirect the stdin/stdout/sderr of a container to
> external clients.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.hpp 272052ddf85b50f817a110a9a83566b011598985

>   src/slave/containerizer/mesos/containerizer.cpp ec4ae32485a7ab6c9f73c512004d1220482a188e

>   src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp PRE-CREATION 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 2f21b49535856186e153cd299dd1eda11495fa17

>   src/tests/containerizer/mesos_containerizer_tests.cpp 5aae23b1b470d5323ecc21fb5df7ad8ae2498dfa

> 
> Diff: https://reviews.apache.org/r/53704/diff/
> 
> 
> Testing
> -------
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


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