----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59949/#review177644 -----------------------------------------------------------
Looks good modulo comments from Jie. Mostly minor style nits. src/launcher/default_executor.cpp Lines 83 (patched) <https://reviews.apache.org/r/59949/#comment251333> One more new line after this. src/launcher/default_executor.cpp Lines 367 (patched) <https://reviews.apache.org/r/59949/#comment251499> s/env/environment src/launcher/default_executor.cpp Lines 438 (patched) <https://reviews.apache.org/r/59949/#comment251498> ```cpp // Set the `MESOS_CONTAINER_IP` environment for the task. ``` src/launcher/default_executor.cpp Lines 441-445 (patched) <https://reviews.apache.org/r/59949/#comment251497> Can we do this after L372 to make them aligned with the code comment earlier? It looks more readable that way. Here, we can just copy the environment into the `CommandInfo` once you make the earlier change. src/launcher/default_executor.cpp Lines 447 (patched) <https://reviews.apache.org/r/59949/#comment251496> Also, in addition to the verbosity, can we do it only once since it's the same for all the tasks in the task group? See my earlier comments around moving this block outside. Also, use `'MESOS_CONTAINER_IP'` in quotes inside the logline. - Anand Mazumdar On June 9, 2017, 6:39 p.m., Avinash sridharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59949/ > ----------------------------------------------------------- > > (Updated June 9, 2017, 6:39 p.m.) > > > Review request for mesos, Anand Mazumdar and Jie Yu. > > > Bugs: MESOS-7631 > https://issues.apache.org/jira/browse/MESOS-7631 > > > Repository: mesos > > > Description > ------- > > Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`. > > > Diffs > ----- > > src/launcher/default_executor.cpp 1a60e3bb2b0e8312b349cddedb4cd55716c1b574 > > > Diff: https://reviews.apache.org/r/59949/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Avinash sridharan > >
