----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51623/#review149269 -----------------------------------------------------------
src/cli/execute.cpp (lines 309 - 313) <https://reviews.apache.org/r/51623/#comment216838> as discussed offline, this should be refactored as ``` CommandScheduler(const FrameworkInfo& framework, const string& master, const Option<Credential>& credential, const Option<Duration>& killAfter, const Option<TaskInfo> task, const Option<TaskGroupInfo> taskGroup); ``` src/cli/execute.cpp (lines 422 - 426) <https://reviews.apache.org/r/51623/#comment216836> this is the wrong place to do this check. should be check when the flags are loaded. src/cli/execute.cpp (lines 538 - 555) <https://reviews.apache.org/r/51623/#comment216840> don't need to do these validations here. master will validate it. src/cli/execute.cpp (lines 569 - 647) <https://reviews.apache.org/r/51623/#comment216844> no need for this if you take TaskGroupInfo as JSON. src/cli/execute.cpp (line 867) <https://reviews.apache.org/r/51623/#comment216845> mesos-default-executor src/cli/execute.cpp (lines 915 - 919) <https://reviews.apache.org/r/51623/#comment216846> instead of 2 different offers functions just have one ``` offers(const vector<Offer>& offers) { // Get resources needed for task or task group. // If resources are available in the offer // -- Accept and launch task (or) // -- Accept and launch task group // Decline if offered resources do not match requirement. } ``` src/cli/execute.cpp (line 1029) <https://reviews.apache.org/r/51623/#comment216847> once command scheduler is refactored, this method might not belong to this class. just make it a static function that takes the required arguments. do this as part of the review that will refactor the command scheduler. - Vinod Kone On Sept. 16, 2016, 6:58 p.m., Abhishek Dasgupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51623/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2016, 6:58 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 c9f56af7f37d5b79b51f350d6c533714c170e889 > > 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 > >
