Marco Massenzio created MESOS-3142:
--------------------------------------

             Summary: As a Developer I want a better way to run shell commands
                 Key: MESOS-3142
                 URL: https://issues.apache.org/jira/browse/MESOS-3142
             Project: Mesos
          Issue Type: Story
          Components: stout
    Affects Versions: 0.23.0
            Reporter: Benjamin Hindman
            Assignee: Marco Massenzio


When reviewing the code in [r/36425|https://reviews.apache.org/r/36425/] 
[~benjaminhindman] noticed that there is a better abstraction that is possible 
to introduce for {{os::shell()}} that will simplify the caller's life.

Instead of having to handle all possible outcomes, we propose to refactor 
{{os::shell()}} as follows:

{code}
/**
 * Returns the output from running the specified command with the shell.
 */
Try<std::string> shell(const string& command)
{
  // Actually handle the WIFEXITED, WIFSIGNALED here!
}
{code}

where the returned string is {{stdout}} and, should the program be signaled, or 
exit with a non-zero exit code, we will simply return a {{Failure}} with an 
error message that will encapsulate both the returned/signaled state, and, 
possibly {{stderr}}.

And some test driven development:
{code}
EXPECT_ERROR(os::shell("false"));
EXPECT_SOME(os::shell("true"));

EXPECT_SOME_EQ("hello world", os::shell("echo hello world"));
{code}

Alternatively, the caller can ask to have {{stderr}} conflated with {{stdout}}:
{code}
Try<string> outAndErr = os::shell("myCmd --foo 2>&1");
{code}

However, {{stderr}} will be ignored by default:
{code}
// We don't read standard error by default.
EXPECT_SOME_EQ("", os::shell("echo hello world 1>&2"));

// We don't even read stderr if something fails (to return in Try::error).
Try<string> output = os::shell("echo hello world 1>&2 && false");
EXPECT_ERROR(output);
EXPECT_FALSE(strings::contains(output.error(), "hello world"));
{code}

An analysis of existing usage shows that in almost all cases, the caller only 
cares {{if not error}}; in fact, the actual exit code is read only once, and 
even then, in a test case.

We believe this will simplify the API to the caller, and will significantly 
reduce the length and complexity at the calling sites (<6 LOC against the 
current 20+).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to