Uploaded image for project: 'Mesos'
  1. Mesos
  2. MESOS-3142

As a Developer I want a better way to run shell commands

    XMLWordPrintableJSON

    Details

    • Type: Story
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.24.0
    • Component/s: stout
    • Sprint:
      Mesosphere Sprint 16
    • Story Points:
      2

      Description

      When reviewing the code in r/36425 Benjamin Hindman 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:

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

      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:

      EXPECT_ERROR(os::shell("false"));
      EXPECT_SOME(os::shell("true"));
      
      EXPECT_SOME_EQ("hello world", os::shell("echo hello world"));
      

      Alternatively, the caller can ask to have stderr conflated with stdout:

      Try<string> outAndErr = os::shell("myCmd --foo 2>&1");
      

      However, stderr will be ignored by default:

      // 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"));
      

      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+).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                marco-mesos Marco Massenzio
                Reporter:
                benjaminhindman Benjamin Hindman
                Shepherd:
                Benjamin Hindman
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: