You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2016/12/12 23:42:13 UTC
Review Request 54681: Used None() for some IOSwitchboardServer flags.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54681/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier and Kevin Klues.
Repository: mesos
Description
-------
Used None() for some IOSwitchboardServer flags.
Diffs
-----
src/slave/containerizer/mesos/io/switchboard.hpp fb720f063396790a336170dcb4b103c844c8d2dc
src/slave/containerizer/mesos/io/switchboard.cpp 210556f6ea60364a6332a07f294331b4d3457ff0
src/slave/containerizer/mesos/io/switchboard_main.cpp 8c4b30aa1a09d3d59f0dd9e81989cd9f3eef89dd
Diff: https://reviews.apache.org/r/54681/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 54681: Used None() for some IOSwitchboardServer
flags.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54681/#review158927
-----------------------------------------------------------
I would reword the commit message to say:
```
Made some IOSwitchboardServer flags optional.
Although these flags are now optional, we actually require them to be set when executing the mesos-io-switchbaord binary. We add a check to make sure they are set properly.
```
src/slave/containerizer/mesos/io/switchboard_main.cpp (line 76)
<https://reviews.apache.org/r/54681/#comment229768>
Given these new semantics, I would probably print the whole usage string and list the exact set of flags that were missing.
As it is written now, it wouldn't be obvious that a flag was missing.
- Kevin Klues
On Dec. 12, 2016, 11:43 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54681/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 11:43 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Kevin Klues.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Used None() for some IOSwitchboardServer flags.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.hpp fb720f063396790a336170dcb4b103c844c8d2dc
> src/slave/containerizer/mesos/io/switchboard.cpp 210556f6ea60364a6332a07f294331b4d3457ff0
> src/slave/containerizer/mesos/io/switchboard_main.cpp 8c4b30aa1a09d3d59f0dd9e81989cd9f3eef89dd
>
> Diff: https://reviews.apache.org/r/54681/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 54681: Made some IOSwitchboardServer flags
optional.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54681/#review158934
-----------------------------------------------------------
Ship it!
I'm fine with shipping this as is. It might be nice to print all missing flags at once though so that you don't have to run it mutliple times to catch all of the missing flags. I think it's fine for now though.
- Kevin Klues
On Dec. 13, 2016, 12:11 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54681/
> -----------------------------------------------------------
>
> (Updated Dec. 13, 2016, 12:11 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Kevin Klues.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Although these flags are now optional, we actually require them to be
> set when executing the mesos-io-switchbaord binary. We add a check to
> make sure they are set properly.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.hpp fb720f063396790a336170dcb4b103c844c8d2dc
> src/slave/containerizer/mesos/io/switchboard.cpp 210556f6ea60364a6332a07f294331b4d3457ff0
> src/slave/containerizer/mesos/io/switchboard_main.cpp 8c4b30aa1a09d3d59f0dd9e81989cd9f3eef89dd
>
> Diff: https://reviews.apache.org/r/54681/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 54681: Made some IOSwitchboardServer flags
optional.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54681/
-----------------------------------------------------------
(Updated Dec. 13, 2016, 12:11 a.m.)
Review request for mesos, Benjamin Bannier and Kevin Klues.
Changes
-------
Addressed Kevin's comments.
Summary (updated)
-----------------
Made some IOSwitchboardServer flags optional.
Repository: mesos
Description (updated)
-------
Although these flags are now optional, we actually require them to be
set when executing the mesos-io-switchbaord binary. We add a check to
make sure they are set properly.
Diffs (updated)
-----
src/slave/containerizer/mesos/io/switchboard.hpp fb720f063396790a336170dcb4b103c844c8d2dc
src/slave/containerizer/mesos/io/switchboard.cpp 210556f6ea60364a6332a07f294331b4d3457ff0
src/slave/containerizer/mesos/io/switchboard_main.cpp 8c4b30aa1a09d3d59f0dd9e81989cd9f3eef89dd
Diff: https://reviews.apache.org/r/54681/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 54681: Used None() for some IOSwitchboardServer
flags.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54681/
-----------------------------------------------------------
(Updated Dec. 12, 2016, 11:43 p.m.)
Review request for mesos, Benjamin Bannier and Kevin Klues.
Repository: mesos
Description
-------
Used None() for some IOSwitchboardServer flags.
Diffs (updated)
-----
src/slave/containerizer/mesos/io/switchboard.hpp fb720f063396790a336170dcb4b103c844c8d2dc
src/slave/containerizer/mesos/io/switchboard.cpp 210556f6ea60364a6332a07f294331b4d3457ff0
src/slave/containerizer/mesos/io/switchboard_main.cpp 8c4b30aa1a09d3d59f0dd9e81989cd9f3eef89dd
Diff: https://reviews.apache.org/r/54681/diff/
Testing
-------
make check
Thanks,
Jie Yu