You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2015/11/05 22:40:47 UTC
Re: Review Request 39769: Fix MESOS-3810 - Include ContainerInfo in
command ExecutorInfo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39769/#review105283
-----------------------------------------------------------
Can you fill in the "testing done" description as well?
Further, I am wondering if we should
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/39769/#comment163794>
Let's rebase it on top of https://reviews.apache.org/r/39866/. That way, this change will disappear.
src/tests/slave_tests.cpp (line 576)
<https://reviews.apache.org/r/39769/#comment163795>
Should we update the test name to make it more verbose `GetExecutorInfoForTaskWithContainer`?
src/tests/slave_tests.cpp (line 626)
<https://reviews.apache.org/r/39769/#comment163829>
s/0.25.0/0.26.0/
src/tests/slave_tests.cpp (lines 640 - 643)
<https://reviews.apache.org/r/39769/#comment163830>
I don't think doing it in two steps adds any value. Let's just use something like:
```
Try<MesosContainerizer*> containerizer =
641
MesosContainerizer::create(flags, false, &fetcher);
642
CHECK_SOME(containerizer);
```
We can then replace `containerizer` with `containerizer.get()` below.
src/tests/slave_tests.cpp (lines 693 - 695)
<https://reviews.apache.org/r/39769/#comment163831>
Can we insert the Jira number here for reference?
- Kapil Arya
On Oct. 29, 2015, 6:15 p.m., Spike Curtis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39769/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 6:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Kapil Arya.
>
>
> Bugs: MESOS-3810
> https://issues.apache.org/jira/browse/MESOS-3810
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Always copy ContainerInfo to ExecutorInfo
>
> Remove MesosContainerizer check for TaskInfo->ContainerInfo
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 91e4ea3a907ad165c359e7422135138737e14085
> src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5
> src/tests/slave_tests.cpp 91dbdba56c7d3a374e56be92d88c0b367c7a2e1c
>
> Diff: https://reviews.apache.org/r/39769/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Spike Curtis
>
>
Re: Review Request 39769: Fix MESOS-3810 - Include ContainerInfo in
command ExecutorInfo
Posted by Spike Curtis <sp...@metaswitch.com>.
> On Nov. 5, 2015, 9:40 p.m., Kapil Arya wrote:
> > src/tests/slave_tests.cpp, lines 693-695
> > <https://reviews.apache.org/r/39769/diff/1/?file=1112436#file1112436line693>
> >
> > Can we insert the Jira number here for reference?
Is there a JIRA tracking this?
- Spike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39769/#review105283
-----------------------------------------------------------
On Oct. 29, 2015, 10:15 p.m., Spike Curtis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39769/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 10:15 p.m.)
>
>
> Review request for mesos, Jie Yu and Kapil Arya.
>
>
> Bugs: MESOS-3810
> https://issues.apache.org/jira/browse/MESOS-3810
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Always copy ContainerInfo to ExecutorInfo
>
> Remove MesosContainerizer check for TaskInfo->ContainerInfo
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 91e4ea3a907ad165c359e7422135138737e14085
> src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5
> src/tests/slave_tests.cpp 91dbdba56c7d3a374e56be92d88c0b367c7a2e1c
>
> Diff: https://reviews.apache.org/r/39769/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Spike Curtis
>
>