You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2014/06/04 01:46:59 UTC

Review Request 22224: Added an abstraction for launching operations in a subprocess.

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

Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.


Bugs: MESOS-1446
    https://issues.apache.org/jira/browse/MESOS-1446


Repository: mesos-git


Description
-------

Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:

1) Code duplication (passing arguments, main function, etc.)
2) Split logic into multiple files (not good for readability)

To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:

class CustomizedOperation : public Operation
{
public:
  class Flags : public flags::FlagsBase
  {
    Flags();
    Option<string> arg1;
    Option<int> arg2;
  };

protected:
  virtual string name() const { return "customized_operation"; }

  virtual int execute()
  {
    if (arg1.isNone()) { return 1; }
    if (arg2.isNone()) { return 1; }

    do_something(arg1.get(), arg2.get());
  }
};

In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:

CustomizedOperation operation;
operation.flags.arg1 = "arg1";
operation.flags.arg2 = "arg2";

operation.launch(flags.launcher_dir + "/mesos-launcher");

We will handle the flags passing and parsing for you under the hood.


Diffs
-----

  src/Makefile.am ffde59b 
  src/launcher/launcher.hpp PRE-CREATION 
  src/launcher/launcher.cpp PRE-CREATION 
  src/launcher/main.cpp PRE-CREATION 
  src/tests/launcher_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/22224/diff/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Jie Yu <yu...@gmail.com>.

> On June 4, 2014, 6:31 p.m., Dominic Hamon wrote:
> > src/tests/launcher_tests.cpp, line 70
> > <https://reviews.apache.org/r/22224/diff/1/?file=603296#file603296line70>
> >
> >     we should be able to inject this. we can start with a default for production and have tests have access to a method that can override it.

Discussed with Dominic offline, he proposed a way to kill 'path' which I liked a lot. Here is the plan:

We will provide a 'launcher::initialize(const std::string& path);'. The 'path' here is the directory I can find the mesos-launcher binary. This 'initialize' should be called before launching any operation (otherwise, launch will fail).

So, when the slave initialize, it should call launcher::initialize(flags.launcher_dir);

In tests, we should call launcher::initialize(tests::flags::build_dir + "/src"); in tests/environment.hpp


- Jie


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


On June 3, 2014, 11:46 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 11:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am ffde59b 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22224/#review44737
-----------------------------------------------------------



src/tests/launcher_tests.cpp
<https://reviews.apache.org/r/22224/#comment79235>

    we should be able to inject this. we can start with a default for production and have tests have access to a method that can override it.


- Dominic Hamon


On June 3, 2014, 4:46 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 4:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am ffde59b 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Jie Yu <yu...@gmail.com>.

> On June 4, 2014, 6:13 p.m., Vinod Kone wrote:
> > src/Makefile.am, line 200
> > <https://reviews.apache.org/r/22224/diff/1/?file=603292#file603292line200>
> >
> >     we already use "launcher" in containerizer. can we use some other name for this to avoid confusion? how about operator? or command?
> 
> Jie Yu wrote:
>     Yeah, but we also have src/launcher directory which is totally irrelevant to Launcher in containerizer.

I'll keep the name for now as we already have src/launcher directory (and we have flags.launcher_dir as well) and I think the name is pretty catchy.


- Jie


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


On June 3, 2014, 11:46 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 11:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am ffde59b 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Jie Yu <yu...@gmail.com>.

> On June 4, 2014, 6:13 p.m., Vinod Kone wrote:
> > src/tests/launcher_tests.cpp, line 70
> > <https://reviews.apache.org/r/22224/diff/1/?file=603296#file603296line70>
> >
> >     do the users even need to know the path of the "mesos-launcher"? why can't "operation" figure that out because it is always the same?

We cannot hard coded it because:

1) in test, it is in <build>/src/mesos-launcher (tests::flags::build_dir)
2) in production, it is in <install>/libexec/mesos-launcher (flags.launcher_dir)


> On June 4, 2014, 6:13 p.m., Vinod Kone wrote:
> > src/launcher/launcher.cpp, line 136
> > <https://reviews.apache.org/r/22224/diff/1/?file=603294#file603294line136>
> >
> >     from the environment and command line right?
> >     
> >     the flags are set in the environment afaict. what other flags are expected on the command line?

Well, the 'launcher' can be used from command line as well (e.g., for debugging purpose). In that case, we are parsing the flags from command line.


> On June 4, 2014, 6:13 p.m., Vinod Kone wrote:
> > src/launcher/launcher.hpp, line 37
> > <https://reviews.apache.org/r/22224/diff/1/?file=603293#file603293line37>
> >
> >     I think "Command" reads better here than "Operation"? e.g: subprocess executes a command.

'Command' is also overloaded. I still prefer Operation:) Do you insist?


> On June 4, 2014, 6:13 p.m., Vinod Kone wrote:
> > src/Makefile.am, line 200
> > <https://reviews.apache.org/r/22224/diff/1/?file=603292#file603292line200>
> >
> >     we already use "launcher" in containerizer. can we use some other name for this to avoid confusion? how about operator? or command?

Yeah, but we also have src/launcher directory which is totally irrelevant to Launcher in containerizer.


- Jie


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


On June 3, 2014, 11:46 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 11:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am ffde59b 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22224/#review44728
-----------------------------------------------------------



src/Makefile.am
<https://reviews.apache.org/r/22224/#comment79199>

    we already use "launcher" in containerizer. can we use some other name for this to avoid confusion? how about operator? or command?



src/launcher/launcher.hpp
<https://reviews.apache.org/r/22224/#comment79203>

    I think "Command" reads better here than "Operation"? e.g: subprocess executes a command.



src/launcher/launcher.hpp
<https://reviews.apache.org/r/22224/#comment79200>

    s/returned integer/return value/



src/launcher/launcher.hpp
<https://reviews.apache.org/r/22224/#comment79202>

    s/Syntax/Syntactic/
    
    Also, can you give an example on how this can be used?



src/launcher/launcher.hpp
<https://reviews.apache.org/r/22224/#comment79204>

    s/TestOperation/ShellOperation/
    
    or
    
    s/TestOperation/ShellCommand/



src/launcher/launcher.cpp
<https://reviews.apache.org/r/22224/#comment79205>

    s/tools/operations/



src/launcher/launcher.cpp
<https://reviews.apache.org/r/22224/#comment79207>

    const?



src/launcher/launcher.cpp
<https://reviews.apache.org/r/22224/#comment79219>

    from the environment and command line right?
    
    the flags are set in the environment afaict. what other flags are expected on the command line?



src/tests/launcher_tests.cpp
<https://reviews.apache.org/r/22224/#comment79209>

    do the users even need to know the path of the "mesos-launcher"? why can't "operation" figure that out because it is always the same?


- Vinod Kone


On June 3, 2014, 11:46 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 11:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am ffde59b 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22224/#review44845
-----------------------------------------------------------



src/launcher/launcher.hpp
<https://reviews.apache.org/r/22224/#comment79410>

    This should be:
    
    extern const char* DEFAULT_EXECUTABLE;
    
    as std::string is non-POD.



src/launcher/launcher.cpp
<https://reviews.apache.org/r/22224/#comment79412>

    const char DEFAULT_EXECUTABLE[] = "mesos-launcher";
    
    same for LAUNCHER_PREFIX


- Dominic Hamon


On June 5, 2014, 10:33 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 5, 2014, 10:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am c3ecb94 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/environment.cpp 3e10508 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22224/#review44874
-----------------------------------------------------------

Ship it!


Lets change EXIT macro to do _exit() instead of exit() to side-step global non-POD destruction issues. If/when we decide to change the code base to disallow global non-POD we can do a sweep and fix.

- Vinod Kone


On June 5, 2014, 5:33 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22224/
> -----------------------------------------------------------
> 
> (Updated June 5, 2014, 5:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-1446
>     https://issues.apache.org/jira/browse/MESOS-1446
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:
> 
> 1) Code duplication (passing arguments, main function, etc.)
> 2) Split logic into multiple files (not good for readability)
> 
> To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:
> 
> class CustomizedOperation : public Operation
> {
> public:
>   class Flags : public flags::FlagsBase
>   {
>     Flags();
>     Option<string> arg1;
>     Option<int> arg2;
>   };
> 
> protected:
>   virtual string name() const { return "customized_operation"; }
> 
>   virtual int execute()
>   {
>     if (arg1.isNone()) { return 1; }
>     if (arg2.isNone()) { return 1; }
> 
>     do_something(arg1.get(), arg2.get());
>   }
> };
> 
> In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:
> 
> CustomizedOperation operation;
> operation.flags.arg1 = "arg1";
> operation.flags.arg2 = "arg2";
> 
> operation.launch(flags.launcher_dir + "/mesos-launcher");
> 
> We will handle the flags passing and parsing for you under the hood.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am c3ecb94 
>   src/launcher/launcher.hpp PRE-CREATION 
>   src/launcher/launcher.cpp PRE-CREATION 
>   src/launcher/main.cpp PRE-CREATION 
>   src/tests/environment.cpp 3e10508 
>   src/tests/launcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22224/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22224/
-----------------------------------------------------------

(Updated June 5, 2014, 5:33 p.m.)


Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.


Changes
-------

Renamed 'configure' to 'setDefaultPath' as it is like PATH in linux. Also made it thread-safe.

Allowed the user to override the default path and executable (useful when a user is creating his own executable).


Bugs: MESOS-1446
    https://issues.apache.org/jira/browse/MESOS-1446


Repository: mesos-git


Description
-------

Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:

1) Code duplication (passing arguments, main function, etc.)
2) Split logic into multiple files (not good for readability)

To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:

class CustomizedOperation : public Operation
{
public:
  class Flags : public flags::FlagsBase
  {
    Flags();
    Option<string> arg1;
    Option<int> arg2;
  };

protected:
  virtual string name() const { return "customized_operation"; }

  virtual int execute()
  {
    if (arg1.isNone()) { return 1; }
    if (arg2.isNone()) { return 1; }

    do_something(arg1.get(), arg2.get());
  }
};

In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:

CustomizedOperation operation;
operation.flags.arg1 = "arg1";
operation.flags.arg2 = "arg2";

operation.launch(flags.launcher_dir + "/mesos-launcher");

We will handle the flags passing and parsing for you under the hood.


Diffs (updated)
-----

  src/Makefile.am c3ecb94 
  src/launcher/launcher.hpp PRE-CREATION 
  src/launcher/launcher.cpp PRE-CREATION 
  src/launcher/main.cpp PRE-CREATION 
  src/tests/environment.cpp 3e10508 
  src/tests/launcher_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/22224/diff/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 22224: Added an abstraction for launching operations in a subprocess.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22224/
-----------------------------------------------------------

(Updated June 5, 2014, 5:57 a.m.)


Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone.


Changes
-------

Vinod and Dominic's comments.


Bugs: MESOS-1446
    https://issues.apache.org/jira/browse/MESOS-1446


Repository: mesos-git


Description
-------

Currently, whenever we want something to be executed in a subprocess, we create a separate binary (e.g., mesos-fetcher, mesos-executor, mesos-usage, etc.). This has several drawbacks:

1) Code duplication (passing arguments, main function, etc.)
2) Split logic into multiple files (not good for readability)

To solve that, I created a generic 'launcher' for 'Operation's. Users can define their own 'Operation' and 'launch' it in a subprocess. For instance:

class CustomizedOperation : public Operation
{
public:
  class Flags : public flags::FlagsBase
  {
    Flags();
    Option<string> arg1;
    Option<int> arg2;
  };

protected:
  virtual string name() const { return "customized_operation"; }

  virtual int execute()
  {
    if (arg1.isNone()) { return 1; }
    if (arg2.isNone()) { return 1; }

    do_something(arg1.get(), arg2.get());
  }
};

In your code, if you want to execute 'CustomizedOperation' in a subprocess, you can just launch it:

CustomizedOperation operation;
operation.flags.arg1 = "arg1";
operation.flags.arg2 = "arg2";

operation.launch(flags.launcher_dir + "/mesos-launcher");

We will handle the flags passing and parsing for you under the hood.


Diffs (updated)
-----

  src/Makefile.am c3ecb94 
  src/launcher/launcher.hpp PRE-CREATION 
  src/launcher/launcher.cpp PRE-CREATION 
  src/launcher/main.cpp PRE-CREATION 
  src/tests/environment.cpp 3e10508 
  src/tests/launcher_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/22224/diff/


Testing
-------

make check


Thanks,

Jie Yu