mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.
Date Thu, 27 Apr 2017 23:15:17 GMT

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




src/slave/containerizer/mesos/containerizer.hpp
Lines 366 (patched)
<https://reviews.apache.org/r/58262/#comment246304>

    I would make this `Option<Environment>` and mention in the comment that this field
won't be set for orphan containers.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1386-1390 (original), 1417-1421 (patched)
<https://reviews.apache.org/r/58262/#comment246309>

    For the sake of consistency, i'd still prefer include agent generated environment variables
in the final result.
    
    For debug container, the list is currently empty so it does not really matter. But just
in case in the future that the agent wants to add some environment variables for debug container.
    
    Please update the comments above as well.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1468-1469 (patched)
<https://reviews.apache.org/r/58262/#comment246307>

    Should we return failure here? If we launch a debug container later under this container,
will it silently miss some environment variables?



src/slave/containerizer/mesos/containerizer.cpp
Lines 1468-1469 (patched)
<https://reviews.apache.org/r/58262/#comment246308>

    Should we return failure here? If we launch a debug container later under this container,
will it silently miss some environment variables?



src/slave/containerizer/mesos/paths.cpp
Lines 372 (patched)
<https://reviews.apache.org/r/58262/#comment246310>

    I'd kill this line.


- Jie Yu


On April 27, 2017, 10:11 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58262/
> -----------------------------------------------------------
> 
> (Updated April 27, 2017, 10:11 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6782
>     https://issues.apache.org/jira/browse/MESOS-6782
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 29a99f33e646593127b9dc126f398f7bca88e21d

>   src/slave/containerizer/mesos/containerizer.cpp b58baed64480e22f640a4852537f85922ed382ae

>   src/slave/containerizer/mesos/paths.hpp d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
>   src/slave/containerizer/mesos/paths.cpp ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 29007898ec04e922266068a8519731b13d351a82

> 
> 
> Diff: https://reviews.apache.org/r/58262/diff/5/
> 
> 
> Testing
> -------
> 
> See https://reviews.apache.org/r/58718/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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