You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@gmail.com> on 2016/01/06 15:36:00 UTC
Review Request 41930: Added "TeardownFramework" to ACL protobuf.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41930/
-----------------------------------------------------------
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
Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41930/#review115594
-----------------------------------------------------------
Ship it!
Ship It!
- Adam B
On Jan. 13, 2016, 11:51 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41930/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:51 p.m.)
>
>
> Review request for mesos and Adam B.
>
>
> Bugs: MESOS-4154
> https://issues.apache.org/jira/browse/MESOS-4154
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added "TeardownFramework" to ACL protobuf.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/authorizer.proto 84727e66dd14be9a25705ab1141e92eee72fae50
>
> Diff: https://reviews.apache.org/r/41930/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41930/
-----------------------------------------------------------
(Updated 一月 14, 2016, 7:51 a.m.)
Review request for mesos and Adam B.
Bugs: MESOS-4154
https://issues.apache.org/jira/browse/MESOS-4154
Repository: mesos
Description (updated)
-------
Added "TeardownFramework" to ACL protobuf.
Diffs (updated)
-----
include/mesos/authorizer/authorizer.proto 84727e66dd14be9a25705ab1141e92eee72fae50
Diff: https://reviews.apache.org/r/41930/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 11, 2016, 6:44 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, lines 147-148
> > <https://reviews.apache.org/r/41930/diff/1/?file=1184556#file1184556line147>
> >
> > What is the expected behavior if both are set? Error? Union?
Yes, Union.
> On 一月 11, 2016, 6:44 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, line 148
> > <https://reviews.apache.org/r/41930/diff/1/?file=1184556#file1184556line148>
> >
> > This is never used? Or is that in a subsequent patch?
subsequent patch.
> On 一月 11, 2016, 6:44 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, line 77
> > <https://reviews.apache.org/r/41930/diff/1/?file=1184558#file1184558line77>
> >
> > Did you mean to iterate through `shutdown_frameworks()` or `teardown_frameworks()`? Or both?
iterate both in the dependency patch.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41930/#review113687
-----------------------------------------------------------
On 一月 6, 2016, 2:35 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41930/
> -----------------------------------------------------------
>
> (Updated 一月 6, 2016, 2:35 p.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
>
>
Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
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
>
>