----------------------------------------------------------- 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 > >
