mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.
Date Mon, 11 Jan 2016 06:44:23 GMT

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



include/mesos/authorizer/authorizer.proto (line 62)
<https://reviews.apache.org/r/41930/#comment174441>

    I don't think you can safely rename a protobuf message type like this. You can rename
a field, since it has an id to uniquely identify it, but I believe you need to add a new (nearly
identical) message type to support the change to TeardownFramework. Please correct me if you've
found evidence indicating otherwise.
    Have you tried testing this change across versions, where an authorizer compiles against
the old version but the master uses the new version? Or vice versa?



include/mesos/authorizer/authorizer.proto (lines 147 - 148)
<https://reviews.apache.org/r/41930/#comment174442>

    What is the expected behavior if both are set? Error? Union?



include/mesos/authorizer/authorizer.proto (line 148)
<https://reviews.apache.org/r/41930/#comment174444>

    This is never used? Or is that in a subsequent patch?



src/authorizer/local/authorizer.cpp (line 77)
<https://reviews.apache.org/r/41930/#comment174443>

    Did you mean to iterate through `shutdown_frameworks()` or `teardown_frameworks()`? Or
both?


- Adam B


On Jan. 6, 2016, 6:35 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41930/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2016, 6:35 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4154
>     https://issues.apache.org/jira/browse/MESOS-4154
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch included the following:
> 1) Renamed "ShutdownFramework" to "TeardownFramework".
> 2) Added teardown_frameworks ACL to protobuf.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.hpp f61613948b7b5c5c2118f1782a0c5f8ff7e8e057 
>   include/mesos/authorizer/authorizer.proto 7b729e19484d92be48bbde4dff2c833a4109936e

>   src/authorizer/local/authorizer.hpp 1563c11709c2612350354690b50ceb33d2720f98 
>   src/authorizer/local/authorizer.cpp 1d135fb6906c7050a788cbac9ca2d8164ff064ef 
>   src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5 
>   src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4 
>   src/tests/mesos.hpp 49a4c48e6887e6f0921d96c359746e39be10e222 
>   src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
>   src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 
> 
> Diff: https://reviews.apache.org/r/41930/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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