Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/#review53658 --- Ship it! Did we lose the test? We need that back to Ship It please. src/docker/docker.cpp https://reviews.apache.org/r/25523/#comment93431 I'd love to add a comment for now that explains why this is not discardable but the other future is, and how that correctly works out. In addition, what happened to the test that is testing this discardability? src/docker/docker.cpp https://reviews.apache.org/r/25523/#comment93430 Swap these two please. src/slave/containerizer/docker.cpp https://reviews.apache.org/r/25523/#comment93414 Comment is out of date now. lose - Benjamin Hindman On Sept. 17, 2014, 1:07 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 17, 2014, 1:07 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 17, 2014, 8:23 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs (updated) - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp a8becb8290c8982c64546c7f72d4bfe89f7ca5a1 src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp e6c228a27a9c3cbf2fa0cc58843989c49fa90456 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25523/#review53675
---
Ship it!
src/tests/docker_tests.cpp
https://reviews.apache.org/r/25523/#comment93472
Please move '{' to newline.
src/tests/docker_tests.cpp
https://reviews.apache.org/r/25523/#comment93473
Why isn't all of this just:
future.discard();
AWAIT_DISCARDED(future);
The extra promise-set(Nothing()) is not testing anything new here. Perhaps
the important thing for me to say is that just because we're calling
'future.discard()' does not mean that the future will get discarded, hence
AWAIT_DISCARDED(future) is sufficient.
Minor cleanup/simplification in the test then lets get this committed!
- Benjamin Hindman
On Sept. 17, 2014, 8:23 a.m., Timothy Chen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25523/
---
(Updated Sept. 17, 2014, 8:23 a.m.)
Review request for mesos and Benjamin Hindman.
Repository: mesos-git
Description
---
Review: https://reviews.apache.org/r/25523
Diffs
-
src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5
src/docker/docker.cpp a8becb8290c8982c64546c7f72d4bfe89f7ca5a1
src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a
src/tests/docker_tests.cpp e6c228a27a9c3cbf2fa0cc58843989c49fa90456
Diff: https://reviews.apache.org/r/25523/diff/
Testing
---
make check
Thanks,
Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 17, 2014, 4:45 p.m.) Review request for mesos, Benjamin Hindman and Vinod Kone. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs (updated) - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp a8becb8290c8982c64546c7f72d4bfe89f7ca5a1 src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp e6c228a27a9c3cbf2fa0cc58843989c49fa90456 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/#review53528 --- src/docker/docker.hpp https://reviews.apache.org/r/25523/#comment93215 From an interface perspective I'd prefer that we created an 'Image' abstraction just like we have 'Container', and have Docker::pull return FutureImage where doing a discard on the future is equivalent to cancel. See https://reviews.apache.org/r/24588 for the initial but incomplete version that I had done before 0.20.0. - Benjamin Hindman On Sept. 11, 2014, 6:44 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 11, 2014, 6:44 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 17, 2014, 1:06 a.m.) Review request for drill and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs (updated) - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 17, 2014, 1:07 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 11, 2014, 6:44 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description (updated) --- Review: https://reviews.apache.org/r/25523 Diffs (updated) - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/#review53028 --- Patch looks great! Reviews applied: [25523] All tests passed. - Mesos ReviewBot On Sept. 11, 2014, 6:44 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- (Updated Sept. 11, 2014, 6:44 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25523 Diffs - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
Review Request 25523: Add Docker pull to docker abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25523/ --- Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Add Docker pull to docker abstraction Diffs - src/docker/docker.hpp e7adedb93272209231a3a9aefecfd6ccc7802ff5 src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 Diff: https://reviews.apache.org/r/25523/diff/ Testing --- make check Thanks, Timothy Chen
