mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rojas" <alexan...@mesosphere.io>
Subject Re: Review Request 36424: Created a command executor helper method.
Date Fri, 17 Jul 2015 08:48:15 GMT

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



3rdparty/libprocess/include/process/subprocess.hpp (lines 302 - 304)
<https://reviews.apache.org/r/36424/#comment145922>

    You may ignore this, but I'm not sure if ignoring `cerr` when the command succeds is the
way to go. Most logging systems log unto the `clog` which is mostly an alias for the `cerr`
file descriptor.



3rdparty/libprocess/include/process/subprocess.hpp (line 307)
<https://reviews.apache.org/r/36424/#comment145919>

    I think you should calle it `execute` to follow our concise naming. Moreover, under the
context is hard to misunderstand it.



3rdparty/libprocess/include/process/subprocess.hpp (line 314)
<https://reviews.apache.org/r/36424/#comment145929>

    ranged based for loops are not yet whitelisted, nor there are any occurrence on the codebase.
So I guess it is not yet time to use them.



3rdparty/libprocess/include/process/subprocess.hpp (lines 334 - 335)
<https://reviews.apache.org/r/36424/#comment145927>

    I'm not very sure about doing the `.get()`, since we didn't verify that the objects where
properly constructed (in line 320) or checking that they are not `None()`.


- Alexander Rojas


On July 17, 2015, 7:52 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36424/
> -----------------------------------------------------------
> 
> (Updated July 17, 2015, 7:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Cody Maloney.
> 
> 
> Bugs: MESOS-3035
>     https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-2902
> 
> While researching how to execute an arbitrary script
> to detect the Master IP address, it emerged clearly that
> a helper method to execute an arbitrary command/script on
> a node and obtain either stdout or stderr would have been
> useful and avoided a lot of code repetition.
> 
> This could not be ultimately used for the purpose at hand,
> but I believe it to be useful enough (particularly, to avoid
> people doing "coding by copy&paste" and/or waste time
> researching the same functionality).
> 
> This would also be beneficial in MESOS-2830 and MESOS-2834
> factoring out the remote command execution logic.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 310cb4f8e4e2faa5545dffd196d7490c868bc5d6

>   3rdparty/libprocess/src/tests/subprocess_tests.cpp f6acb204582a9e696c3b09d4e4c543bb052e97d4

> 
> Diff: https://reviews.apache.org/r/36424/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


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