You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2018/09/20 23:13:40 UTC
Review Request 68790: Moved the container ID prefix generation to
`LocalResourceProvider`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68790/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
Bugs: MESOS-9228
https://issues.apache.org/jira/browse/MESOS-9228
Repository: mesos
Description
-------
It is more reasonable to not allow each specific resource provider to
construct their own container ID prefix, otherwise it would be hard to
avoid conflicts. Therefore we now established the convension of how the
prefix is constructed in `LocalResourceProvider`.
Diffs
-----
src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55
src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29
src/resource_provider/local.cpp 801e6c430ed91315d87f8a45b8f3ed128beca4fc
src/resource_provider/storage/provider.cpp 6475f653263337c381b6080695d09c49e5ea8fcf
Diff: https://reviews.apache.org/r/68790/diff/1/
Testing
-------
make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 68790: Moved the container ID prefix generation to
`LocalResourceProvider`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68790/
-----------------------------------------------------------
(Updated Sept. 21, 2018, 7:44 p.m.)
Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
Changes
-------
Addressed Benjamin's comments.
Bugs: MESOS-9228
https://issues.apache.org/jira/browse/MESOS-9228
Repository: mesos
Description
-------
It is more reasonable to not allow each specific resource provider to
construct their own container ID prefix, otherwise it would be hard to
avoid conflicts. Therefore we now established the convension of how the
prefix is constructed in `LocalResourceProvider`.
Diffs (updated)
-----
src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55
src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29
src/resource_provider/local.cpp 801e6c430ed91315d87f8a45b8f3ed128beca4fc
src/resource_provider/storage/provider.cpp 6475f653263337c381b6080695d09c49e5ea8fcf
Diff: https://reviews.apache.org/r/68790/diff/2/
Changes: https://reviews.apache.org/r/68790/diff/1-2/
Testing
-------
make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 68790: Moved the container ID prefix generation to
`LocalResourceProvider`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68790/#review208845
-----------------------------------------------------------
Fix it, then Ship it!
src/resource_provider/local.cpp
Lines 36-41 (original), 36-40 (patched)
<https://reviews.apache.org/r/68790/#comment293073>
This datastructure makes much more sense now, `principal` never really fit here.
src/resource_provider/storage/provider.cpp
Line 196 (original), 180 (patched)
<https://reviews.apache.org/r/68790/#comment293074>
Let's add an explicit assertion here that `claims` always contains that explicit key and not propagate the raw exception. The code doing the initialization is far away, even in a different class.
src/resource_provider/storage/provider.cpp
Lines 2089 (patched)
<https://reviews.apache.org/r/68790/#comment293075>
Ditto.
- Benjamin Bannier
On Sept. 21, 2018, 1:13 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68790/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 1:13 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9228
> https://issues.apache.org/jira/browse/MESOS-9228
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It is more reasonable to not allow each specific resource provider to
> construct their own container ID prefix, otherwise it would be hard to
> avoid conflicts. Therefore we now established the convension of how the
> prefix is constructed in `LocalResourceProvider`.
>
>
> Diffs
> -----
>
> src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55
> src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29
> src/resource_provider/local.cpp 801e6c430ed91315d87f8a45b8f3ed128beca4fc
> src/resource_provider/storage/provider.cpp 6475f653263337c381b6080695d09c49e5ea8fcf
>
>
> Diff: https://reviews.apache.org/r/68790/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>