You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2012/05/22 02:41:57 UTC

Review Request: Added support for running local executor

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

Review request for mesos, Benjamin Hindman and John Sirois.


Summary
-------

Executor launcher now supports copying executor from local file system


Diffs
-----

  include/mesos/mesos.proto ab02b95 
  src/launcher/launcher.cpp 5d8ef6c 

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


Testing
-------

Tested with AB.

Also,
make check.


Thanks,

Vinod


Re: Review Request: Added support for running local executor

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5187/#review8067
-----------------------------------------------------------

Ship it!


- Benjamin


On 2012-05-23 18:04:13, Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5187/
> -----------------------------------------------------------
> 
> (Updated 2012-05-23 18:04:13)
> 
> 
> Review request for mesos, Benjamin Hindman and John Sirois.
> 
> 
> Summary
> -------
> 
> Executor launcher now supports copying executor from local file system
> 
> 
> Diffs
> -----
> 
>   src/common/utils.hpp 1d81e21 
>   src/launcher/launcher.cpp 5d8ef6c 
>   include/mesos/mesos.proto ab02b95 
> 
> Diff: https://reviews.apache.org/r/5187/diff
> 
> 
> Testing
> -------
> 
> Tested with AB.
> 
> Also,
> make check.
> 
> 
> Thanks,
> 
> Vinod
> 
>


Re: Review Request: Added support for running local executor

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

(Updated 2012-05-23 18:04:13.000520)


Review request for mesos, Benjamin Hindman and John Sirois.


Changes
-------

ben's comments


Summary
-------

Executor launcher now supports copying executor from local file system


Diffs (updated)
-----

  src/common/utils.hpp 1d81e21 
  src/launcher/launcher.cpp 5d8ef6c 
  include/mesos/mesos.proto ab02b95 

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


Testing
-------

Tested with AB.

Also,
make check.


Thanks,

Vinod


Re: Review Request: Added support for running local executor

Posted by Vinod Kone <vi...@gmail.com>.

> On 2012-05-23 05:36:47, Benjamin Hindman wrote:
> > include/mesos/mesos.proto, line 271
> > <https://reviews.apache.org/r/5187/diff/1/?file=109764#file109764line271>
> >
> >     Actually, this should just say that one of ExecutorInfo or CommandInfo should be set, rather than mention anything about an "ephemeral" executor for the CommandInfo case.

done


> On 2012-05-23 05:36:47, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 231
> > <https://reviews.apache.org/r/5187/diff/1/?file=109765#file109765line231>
> >
> >     Use utils::os::system.

done..here and everywhere else


> On 2012-05-23 05:36:47, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 237
> > <https://reviews.apache.org/r/5187/diff/1/?file=109765#file109765line237>
> >
> >     Use utils::os::chmod.

doesnt exist. wrote one.


> On 2012-05-23 05:36:47, Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 239
> > <https://reviews.apache.org/r/5187/diff/1/?file=109765#file109765line239>
> >
> >     Wrap this in '{' '}'.

done


- Vinod


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


On 2012-05-22 00:41:57, Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5187/
> -----------------------------------------------------------
> 
> (Updated 2012-05-22 00:41:57)
> 
> 
> Review request for mesos, Benjamin Hindman and John Sirois.
> 
> 
> Summary
> -------
> 
> Executor launcher now supports copying executor from local file system
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ab02b95 
>   src/launcher/launcher.cpp 5d8ef6c 
> 
> Diff: https://reviews.apache.org/r/5187/diff
> 
> 
> Testing
> -------
> 
> Tested with AB.
> 
> Also,
> make check.
> 
> 
> Thanks,
> 
> Vinod
> 
>


Re: Review Request: Added support for running local executor

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5187/#review8052
-----------------------------------------------------------



include/mesos/mesos.proto
<https://reviews.apache.org/r/5187/#comment17506>

    Actually, this should just say that one of ExecutorInfo or CommandInfo should be set, rather than mention anything about an "ephemeral" executor for the CommandInfo case.



src/launcher/launcher.cpp
<https://reviews.apache.org/r/5187/#comment17507>

    Use utils::os::system.



src/launcher/launcher.cpp
<https://reviews.apache.org/r/5187/#comment17508>

    Use utils::os::chmod.



src/launcher/launcher.cpp
<https://reviews.apache.org/r/5187/#comment17509>

    Wrap this in '{' '}'.


- Benjamin


On 2012-05-22 00:41:57, Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5187/
> -----------------------------------------------------------
> 
> (Updated 2012-05-22 00:41:57)
> 
> 
> Review request for mesos, Benjamin Hindman and John Sirois.
> 
> 
> Summary
> -------
> 
> Executor launcher now supports copying executor from local file system
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ab02b95 
>   src/launcher/launcher.cpp 5d8ef6c 
> 
> Diff: https://reviews.apache.org/r/5187/diff
> 
> 
> Testing
> -------
> 
> Tested with AB.
> 
> Also,
> make check.
> 
> 
> Thanks,
> 
> Vinod
> 
>