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



src/cli/execute.cpp (line 237)
<https://reviews.apache.org/r/40371/#comment166033>

    As suggested below, driver->abort() here and print the unsupported 
containerizer.
    
    The advantage is that we only have one place to add or remove 
containerizers.



src/cli/execute.cpp (line 418)
<https://reviews.apache.org/r/40371/#comment166032>

    IMO it's simpler to just have one place to check this that's happening in 
the CommandScheduler code, as you already have a check for mesos or docker.
    
    I think it's not necessary to add checks in both places, because then it's 
not clear when someone else comes and add another containerizer support, that 
why they need to add here in line 417 but also in the resourceOffer callback.
    
    How about just pass the flag in without any checks here, and in the 
CommandScheduler you can detect a unsupported containerizer and abort the 
driver.


- Timothy Chen


On Nov. 17, 2015, 3:01 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2015, 3:01 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a  new option 'containerizer' to disambiguate the two 
> containerizers.
> 
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
> 
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
> 
> 
> Diffs
> -----
> 
>   src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc 
> 
> Diff: https://reviews.apache.org/r/40371/diff/
> 
> 
> Testing
> -------
> 
> tested the two containerizer locally with simple commands.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Reply via email to