Re: Review Request 15542: Offer an execvp like interface for running tasks.

[Ben Mahler]

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



src/examples/python/test_framework.py
<https://reviews.apache.org/r/15542/#comment57419>

    Can you kill this line?



src/launcher/executor.cpp
<https://reviews.apache.org/r/15542/#comment57421>

    s/std::vector/vector/ (Add a using clause above)
    s/std::string/string/



src/launcher/executor.cpp
<https://reviews.apache.org/r/15542/#comment57423>

    I believe you need an std::back_inserter to use std::copy like this, see:
    http://stackoverflow.com/a/5034274
    
    Or more simply:
    args.insert(
        args.begin(),
        command.arguments.begin(),
        command.arguemnts.end());



src/launcher/executor.cpp
<https://reviews.apache.org/r/15542/#comment57425>

    s/std::string/string/
    
    Do you want a space after the colon to print nicely?



src/launcher/executor.cpp
<https://reviews.apache.org/r/15542/#comment57426>

    s/std::vector/vector/



src/launcher/executor.cpp
<https://reviews.apache.org/r/15542/#comment57427>

    s/string/string&/



src/launcher/executor.cpp
<https://reviews.apache.org/r/15542/#comment57428>

    What's this for? c_str is not async signal safe?
    
    The cast is async signal safe, can you move it below?



src/launcher/launcher.cpp
<https://reviews.apache.org/r/15542/#comment57429>

    Ditto my comments from above.



src/launcher/launcher.cpp
<https://reviews.apache.org/r/15542/#comment57430>

    Remove std:: (in general we use using clauses in cpp files).



src/launcher/launcher.cpp
<https://reviews.apache.org/r/15542/#comment57431>

    string&



src/slave/process_isolator.cpp
<https://reviews.apache.org/r/15542/#comment57444>

    Remove std:: qualifiers.



src/slave/process_isolator.cpp
<https://reviews.apache.org/r/15542/#comment57446>

    Ditto on needing a back inserter or just using insert().



src/slave/process_isolator.cpp
<https://reviews.apache.org/r/15542/#comment57447>

    Kill std:: qualifier.



src/slave/process_isolator.cpp
<https://reviews.apache.org/r/15542/#comment57448>

    string&



src/slave/process_isolator.cpp
<https://reviews.apache.org/r/15542/#comment57449>

    Can we do this cast down below where it's used?


- Ben Mahler


On Nov. 25, 2013, 5:13 a.m., Jason Dusek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15542/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2013, 5:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Offer an execvp like interface for running tasks.
> 
> Review: https://reviews.apache.org/r/15542
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 655f86757487ddbe551fdcf53eb793e773ecdd34 
>   src/examples/python/test_framework.py 
> deca48e779ae099424fa73bb9a8ac5c419c5faf1 
>   src/launcher/executor.cpp b73ab479500a7347a38ba53acecfab9229f1080d 
>   src/launcher/launcher.cpp d5ab66704429a95eeb8eda5188e33d8e691221af 
>   src/launcher/main.cpp de64609905ee63096c0173fe7e64a1eafea5d6bf 
>   src/slave/process_isolator.cpp a6e9ed6a654972e8a51a9a033052e02ce44fe3e4 
> 
> Diff: https://reviews.apache.org/r/15542/diff/
> 
> 
> Testing
> -------
> 
> Ran Python test executor and `make check`.
> 
> 
> Thanks,
> 
> Jason Dusek
> 
>