You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gastón Kleiman <ga...@mesosphere.com> on 2017/01/18 19:01:00 UTC
Review Request 55677: Made `AttachContainerOutput/Input` tests use an
existing container ID.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/
-----------------------------------------------------------
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
These tests verify that the Agent returns a 500 if the containerizer
doesn't support the `attach` call.
This chain refactors those methods and make them return a 404 instead of
a 500 if the container can't be found, so we need to pass the ID of an
existing container.
Diffs
-----
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55677/diff/
Testing
-------
`make check` in macOS and Linux.
Thanks,
Gast�n Kleiman
Re: Review Request 55677: Made `AttachContainerOutput/Input` tests
use an existing container ID.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/
-----------------------------------------------------------
(Updated Jan. 19, 2017, 4:19 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
Changes
-------
Addressed feedback.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
These tests verify that the Agent returns a 500 if the containerizer
doesn't support the `attach` call.
This chain refactors those methods and make them return a 404 instead of
a 500 if the container can't be found, so we need to pass the ID of an
existing container.
Diffs (updated)
-----
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55677/diff/
Testing
-------
`make check` in macOS and Linux.
Thanks,
Gast�n Kleiman
Re: Review Request 55677: Made `AttachContainerOutput/Input` tests
use an existing container ID.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/#review162274
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rojas
On Jan. 19, 2017, 11:17 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 11:17 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
>
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55677/diff/
>
>
> Testing
> -------
>
> `make check` in macOS and Linux.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55677: Made `AttachContainerOutput/Input` tests
use an existing container ID.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
> On Jan. 19, 2017, 2:23 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, line 4266
> > <https://reviews.apache.org/r/55677/diff/2/?file=1608897#file1608897line4266>
> >
> > we don't do "Times(1)" since that is the default.
> >
> > just do
> >
> > ```
> > EXPECT_CALL(exec, registered(_, _, _, _));
> >
> > ```
https://reviews.apache.org/r/55722/
- Gast�n
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/#review162287
-----------------------------------------------------------
On Jan. 19, 2017, 4:19 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 4:19 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
>
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55677/diff/
>
>
> Testing
> -------
>
> `make check` in macOS and Linux.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55677: Made `AttachContainerOutput/Input` tests
use an existing container ID.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/#review162287
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/api_tests.cpp (line 4256)
<https://reviews.apache.org/r/55677/#comment233620>
we don't do "Times(1)" since that is the default.
just do
```
EXPECT_CALL(exec, registered(_, _, _, _));
```
src/tests/api_tests.cpp (line 4342)
<https://reviews.apache.org/r/55677/#comment233621>
ditto.
- Vinod Kone
On Jan. 19, 2017, 10:17 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 10:17 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
>
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55677/diff/
>
>
> Testing
> -------
>
> `make check` in macOS and Linux.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55677: Made `AttachContainerOutput/Input` tests
use an existing container ID.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/#review162293
-----------------------------------------------------------
Fix it, then Ship it!
Modulo comments from Vinod.
src/tests/api_tests.cpp (line 4248)
<https://reviews.apache.org/r/55677/#comment233627>
Nit: Remove this? You don't need to ignore subsequent offers as there won't be any due to your task never reaching a terminal state before the driver was stopped.
src/tests/api_tests.cpp (line 4334)
<https://reviews.apache.org/r/55677/#comment233628>
Ditto as above.
- Anand Mazumdar
On Jan. 19, 2017, 10:17 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 10:17 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
>
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55677/diff/
>
>
> Testing
> -------
>
> `make check` in macOS and Linux.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55677: Made `AttachContainerOutput/Input` tests
use an existing container ID.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55677/
-----------------------------------------------------------
(Updated Jan. 19, 2017, 10:17 a.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
Changes
-------
Rebased.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
These tests verify that the Agent returns a 500 if the containerizer
doesn't support the `attach` call.
This chain refactors those methods and make them return a 404 instead of
a 500 if the container can't be found, so we need to pass the ID of an
existing container.
Diffs (updated)
-----
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55677/diff/
Testing
-------
`make check` in macOS and Linux.
Thanks,
Gast�n Kleiman