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



Looking pretty good. Would be great to have a CHANGELOG update here that 
outlines the deprecation and what we're advising users to do in each version.


docs/configuration.md (lines 1133 - 1135)
<https://reviews.apache.org/r/44661/#comment186047>

    We should help the users by adding a corresponding deprecation notice 
within the CHANGELOG, can you include that in this diff please?



src/docker/executor.hpp (lines 55 - 59)
<https://reviews.apache.org/r/44661/#comment186050>

    Looking at this in isolation it's hard to tell why it's deprecated. Could 
you add the similar blurb to the help string? Or expand on the comment?



src/docker/executor.hpp (line 74)
<https://reviews.apache.org/r/44661/#comment186049>

    Just to be clear, is the intention to remove this in 0.30? If so, can you 
say that explicitly? Or are we giving time for users to set kill policies and 
planning to remove it 6 months from 0.29.0?



src/slave/containerizer/docker.cpp (lines 960 - 962)
<https://reviews.apache.org/r/44661/#comment186053>

    How about:
    
    ```
          // TODO(alexr): After the deprecation cycle (started in 0.29.0), 
update
          // this to omit the timeout. Graceful shutdown of the container is not
          // a containerizer responsibility; it is the responsibility of the 
agent
          // in co-operation with the executor. Once `destroy` is called, the
          // container should be destroyed forcefully.
    ```
    
    Ditto below.



src/slave/containerizer/docker.cpp (line 966)
<https://reviews.apache.org/r/44661/#comment186054>

    Hm.. I'm a bit confused about the deprecation taking place here. Could you 
outline the steps in this description? Specifically, what does the 0.28, 0.29, 
and final versions look like and what are we advising users to do in each 
version?
    
    This information would be great to have in the CHANGELOG deprecation 
message I suggested above.



src/slave/flags.cpp (line 532)
<https://reviews.apache.org/r/44661/#comment186051>

    poly?


- Ben Mahler


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44661/
> -----------------------------------------------------------
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4910
>     https://issues.apache.org/jira/browse/MESOS-4910
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Instead, a combination of `executor_shutdown_grace_period`
> agent flag and task kill policies should be used.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44661/diff/
> 
> 
> Testing
> -------
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Reply via email to