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




src/cli/execute.cpp (lines 73 - 74)
<https://reviews.apache.org/r/51623/#comment216902>

    adjust the order here



src/cli/execute.cpp (lines 389 - 395)
<https://reviews.apache.org/r/51623/#comment216920>

    How about:
    
    ```
    CHECK_NE(task.isSome(), taskGroup.isSome())
      << "Either task or task group should be set but not both";
    
    vector<TaskInfo> tasks;
    if (task.isSome()) {
      requiredResources = Resources(task.get().resources());
    } else {
      foreach (const TaskInfo& _task, taskgroup.get().tasks()) {
        requiredResources += Resources(_task.resources());
      }
    }
    ```
    
    Ditto for others



src/cli/execute.cpp (lines 406 - 409)
<https://reviews.apache.org/r/51623/#comment216926>

    I'd like a `CHECK_SOME` here for flatten.
    
    ```
    // Takes resources first from the specified role, then from '*'.
    Try<Resources> flattened =
      Resources(task.get().resources()).flatten(frameworkInfo.role());
    
    // `frameworkInfo.role()` must be valid as it's allowed to register.
    CHECK_SOME(flattened);
    Option<Resources> resources = offered.find(flattened.get());
    ```



src/cli/execute.cpp (line 414)
<https://reviews.apache.org/r/51623/#comment216922>

    This can be updated to `else {}` if you `CHECK_NE` above.
    
    Ditto for others.



src/cli/execute.cpp (lines 419 - 422)
<https://reviews.apache.org/r/51623/#comment216927>

    I'd like a `CHECK_SOME` here for flatten.
    
    ```
    // Takes resources first from the specified role, then from '*'.
    Try<Resources> flattened =
      Resources(_task.resources()).flatten(frameworkInfo.role());
    
    // `frameworkInfo.role()` must be valid as it's allowed to register.
    CHECK_SOME(flattened);
    Option<Resources> resources = offered.find(flattened.get());
    ```



src/cli/execute.cpp (line 444)
<https://reviews.apache.org/r/51623/#comment216914>

    s/if(/if (



src/cli/execute.cpp (line 452)
<https://reviews.apache.org/r/51623/#comment216915>

    4 spaces



src/cli/execute.cpp (line 456)
<https://reviews.apache.org/r/51623/#comment216916>

    4 spaces



src/cli/execute.cpp (line 468)
<https://reviews.apache.org/r/51623/#comment216918>

    s/taskgroup/task group



src/cli/execute.cpp (lines 468 - 469)
<https://reviews.apache.org/r/51623/#comment216919>

    How about
    
    ```
    Submitted task group with tasks [task1, task2] to agent 'agetname'
    ```



src/cli/execute.cpp (line 547)
<https://reviews.apache.org/r/51623/#comment216921>

    break here?



src/cli/execute.cpp (lines 771 - 777)
<https://reviews.apache.org/r/51623/#comment216923>

    Move this to line 758 to fail fast.



src/cli/execute.cpp (line 774)
<https://reviews.apache.org/r/51623/#comment216924>

    s/Either of task or taskgroup can be run at one time/Either task or task 
group should be set but not both



src/cli/execute.cpp (line 929)
<https://reviews.apache.org/r/51623/#comment216925>

    s/Option <TaskInfo>/Option<TaskInfo>


- Guangya Liu


On 九月 17, 2016, 8:56 p.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> -----------------------------------------------------------
> 
> (Updated 九月 17, 2016, 8:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
>     https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated mesos-execute to support task groups.
> 
> 
> Diffs
> -----
> 
>   src/cli/execute.cpp 525c89803ad1b29328420c5925a3e6045e487645 
> 
> Diff: https://reviews.apache.org/r/51623/diff/
> 
> 
> Testing
> -------
> 
> On Ubuntu 16.04:
> sudo make -j4
> 
> and manually ran this command:
> **Successful**
> mesos-execute --master=127.0.0.1:5050 
> --taskgroup=file:///home/abhishek/taskgroup.txt
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Reply via email to