mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anand Mazumdar <an...@apache.org>
Subject Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.
Date Tue, 13 Jun 2017 19:02:37 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message