You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Timothy Chen <tn...@apache.org> on 2014/09/17 03:06:58 UTC
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
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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