Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Nov. 7, 2015, 9:36 a.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51
> > 
> >
> > This is no longer true since we pulled these out of RemotePuller right? 
> > Same as all the other ones.

No its part of RegistryPuller


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105579
---


On Nov. 8, 2015, 4:58 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 8, 2015, 4:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt cbc25e3ea33c47a787d34a7fa8499af3eb0b2c10 
>   src/Makefile.am c479acafc92b7e9d078729d443b0d2f00d40f353 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> bc487d34e4087fa53452d63416d0f31e77060eb0 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp 825b6127dfbc39cb3aea852875eaaab95bac0516 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51
> > 
> >
> > Didn''t you pull these out?
> 
> Jojy Varghese wrote:
> Not sure what you mean.

No its part of the RegistryPuller class.


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---


On Nov. 8, 2015, 4:58 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 8, 2015, 4:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt cbc25e3ea33c47a787d34a7fa8499af3eb0b2c10 
>   src/Makefile.am c479acafc92b7e9d078729d443b0d2f00d40f353 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> bc487d34e4087fa53452d63416d0f31e77060eb0 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp 825b6127dfbc39cb3aea852875eaaab95bac0516 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Nov. 7, 2015, 9:36 a.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 246
> > 
> >
> > Can there be layers that actually have zero content? Docker images 
> > AFAIK has the last layer left empty as a mutable layer, so I'm wondering if 
> > you could actually get empty layer from an image.

Not sure. I tried ubuntu and busybox images and couldnt get a "empty" layer.


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105579
---


On Nov. 8, 2015, 4:58 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 8, 2015, 4:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt cbc25e3ea33c47a787d34a7fa8499af3eb0b2c10 
>   src/Makefile.am c479acafc92b7e9d078729d443b0d2f00d40f353 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> bc487d34e4087fa53452d63416d0f31e77060eb0 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp 825b6127dfbc39cb3aea852875eaaab95bac0516 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 7, 2015, 12:11 a.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

review


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt cbc25e3ea33c47a787d34a7fa8499af3eb0b2c10 
  src/Makefile.am 736f81252712bc4eee2c114535c325777a85a78f 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
bc487d34e4087fa53452d63416d0f31e77060eb0 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
fe6a90fe32364eec8ef923a000db19183603c338 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 6, 2015, 5:36 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
8d90894410cd834edf49a2814d1b616718798fe8 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105512
---



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 408)


I don't think we're adding any value here logging this. Same as onfailed as 
well, let's just leave it to the caller since we don't have much to say.


- Timothy Chen


On Nov. 6, 2015, 5:36 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 6, 2015, 5:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 5, 2015, 4:03 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
8d90894410cd834edf49a2814d1b616718798fe8 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 5, 2015, 11:57 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

review comments


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
8d90894410cd834edf49a2814d1b616718798fe8 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105306
---



src/tests/containerizer/provisioner_docker_tests.cpp (line 1001)


Please update the comment as we discussed in our meeting, to be more 
descriptive what you're actually testing.



src/tests/containerizer/provisioner_docker_tests.cpp (line 1004)


Let's move the setup_server and ASSERTS to the Setup method of 
RegistryClientTest class as we're doing it every time.



src/tests/containerizer/provisioner_docker_tests.cpp (line 1191)


io::read should already do os::nonblock, you don't need to do it agian.



src/tests/containerizer/provisioner_docker_tests.cpp (line 1217)


What are these for?


- Timothy Chen


On Nov. 5, 2015, 4:03 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 5, 2015, 4:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 236
> > 
> >
> > Let's use consistent failure messaging here.
> > If you like to include the layer id please include in both cases, and 
> > also specify the layer as well for all cases.
> 
> Jojy Varghese wrote:
> Not sure i understand.

You have promise->fail here that includes "Failed to download layer", but in 
the else branch you don't.
Also for discarded you don't also print out why it failed, but then in the else 
branch you print out the failure message.

What I hope is that we have a consistent message in the beginning for all cases:
"Failed to download layer '" + layer.layerId + "': " isDiscarded() ? "future 
discarded" : failure()

Does this makes sense?


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105166
---



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 373)


Are you suggesting moving lines 373 and 374 into getRemoteNameFromLocalName 
method?


- Jojy Varghese


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 373
> > 
> >
> > Instead of doing this, how about just create a new Image::Name?
> 
> Jojy Varghese wrote:
> The intent here is that we create  the new image name exactly as the one 
> passed one expect the repository name.

Which is I thought it's weird that we have to also set another name, I would 
thought having a helper function that just gives us the right remote 
Image::Name and we put all the details there is better.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Nov. 4, 2015, 10:05 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 43
> > 
> >
> > This was done in another patch by Ben M (currently in 
> > registry_client.cpp). So i thought the style was safe.

I think you're right, feel free to drop it.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105151
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Nov. 4, 2015, 10:54 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 373
> > 
> >
> > Are you suggesting moving lines 373 and 374 into 
> > getRemoteNameFromLocalName method?

Yes, and let getRemoteNameFromLocalName return a Image::Name instead of a 
string.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105166
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105151
---



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 43)


This was done in another patch by Ben M (currently in registry_client.cpp). 
So i thought the style was safe.


- Jojy Varghese


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 295
> > 
> >
> > Are you trying to check for custom registry?
> > 
> > The Name struct has an optional registry field that you should check 
> > instead:
> > 
> > if (name.has_registry()) {
> >   return Error(.);
> > }
> 
> Jojy Varghese wrote:
> The issue is that we dont know what is being passed as Image::Name. Is it 
> the local name or canonical name. If its local name, checking for "/" is 
> sufficient. The idea is that we dont accept any paths in the name.

I'm not sure I really understand the difference between local and canonical, 
but whatever image name people have been using with docker client is what the 
name we're expecting the user to give us as well. I believe it's local name, 
and we don't need to really expect another format.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51
> > 
> >
> > Didn''t you pull these out?

Not sure what you mean.


> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 129
> > 
> >
> > Move the timeout after into the RemotePullerProcess.

Since RemotePullerProcess does many sub-steps, how do I set  a timer for all of 
the steps aggregated?


> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 133
> > 
> >
> > Let the caller LOG instead of us since we already forward all necessary 
> > information to the caller.

I wanted context of the failure logged out at the source of it to help debug 
issues.


> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 236
> > 
> >
> > Let's use consistent failure messaging here.
> > If you like to include the layer id please include in both cases, and 
> > also specify the layer as well for all cases.

Not sure i understand.


> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 295
> > 
> >
> > Are you trying to check for custom registry?
> > 
> > The Name struct has an optional registry field that you should check 
> > instead:
> > 
> > if (name.has_registry()) {
> >   return Error(.);
> > }

The issue is that we dont know what is being passed as Image::Name. Is it the 
local name or canonical name. If its local name, checking for "/" is 
sufficient. The idea is that we dont accept any paths in the name.


> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 373
> > 
> >
> > Instead of doing this, how about just create a new Image::Name?

The intent here is that we create  the new image name exactly as the one passed 
one expect the repository name.


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105167
---



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 129)


Then will the timeout property be that of the store?


- Jojy Varghese


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---



src/Makefile.am (line 564)


We use tab indentation in the Makefilm.am



src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp (line 57)


I think this is redudant.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp (line 69)


We talked about removing these right? And it's not matching the actual 
params



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 43)


We don't use alias namespaces tpyically.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 51)


Didn''t you pull these out?



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 129)


Move the timeout after into the RemotePullerProcess.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 133)


Let the caller LOG instead of us since we already forward all necessary 
information to the caller.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 213)


Remove extra space



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 216)


Fix indentation



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 236)


Let's use consistent failure messaging here.
If you like to include the layer id please include in both cases, and also 
specify the layer as well for all cases.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 295)


Are you trying to check for custom registry?

The Name struct has an optional registry field that you should check 
instead:

if (name.has_registry()) {
  return Error(.);
}



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 303)


Can you rebase on all your refactoring patches?



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 373)


Instead of doing this, how about just create a new Image::Name?



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 380)


Remove extra space between ' and :



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 385)


ditto



src/slave/flags.cpp (line 118)


This is only used in the remote puller for now, so perhaps say "for pulling 
images from the Docker registry"



src/slave/flags.cpp (line 123)


How about "Default Docker registry server host"



src/slave/flags.cpp (line 128)


Default Docker registry server port


- Timothy Chen


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 129
> > 
> >
> > Move the timeout after into the RemotePullerProcess.
> 
> Jojy Varghese wrote:
> Since RemotePullerProcess does many sub-steps, how do I set  a timer for 
> all of the steps aggregated?

I think if we are going to have a generic puller timeout, let's put the after 
in the store then, and discard the future outside.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
---


On Nov. 4, 2015, 6:09 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Nov. 4, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 87d80026939a326bd1169f46906e36d6ef19f546 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> ad57d8592c5f9df5115b575855ed2e99a0597359 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 8d90894410cd834edf49a2814d1b616718798fe8 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 4, 2015, 11:27 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
8d90894410cd834edf49a2814d1b616718798fe8 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 4, 2015, 6:09 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

added discard semantics


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
8d90894410cd834edf49a2814d1b616718798fe8 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Nov. 1, 2015, 3:38 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
99c2af7d06dd7908e8d77e1bfb3e20622fb3eb4c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 27, 2015, 2:27 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

collect untar futures.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
822aa77d0be0797ab62a5798527618b2cc4f6bf0 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 27, 2015, 3:42 a.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

moved untar to after all the downloads are done.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
822aa77d0be0797ab62a5798527618b2cc4f6bf0 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Klaus Ma]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review103930
---


Please link this RR to ticket.

- Klaus Ma


On Oct. 24, 2015, 12:57 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 24, 2015, 12:57 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
>   src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 1d3377e7462908246dfac90aa0c56314406529c9 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 471783d88b73b62afacac3d7952ebb5d5f442097 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 822aa77d0be0797ab62a5798527618b2cc4f6bf0 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 23, 2015, 4:57 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

minor changes.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
822aa77d0be0797ab62a5798527618b2cc4f6bf0 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 13, 2015, 7:19 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
  src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > 
> >
> > I thought you wanted to move this to somewhere shared? We can create a 
> > base puller class and move this there.
> 
> Jojy Varghese wrote:
> Thought about it a little more and realized that the functionality of 
> "untar a tarball into a dierctory" should belong in a common place like 
> libprocess. Its not a function of puller but maybe a Tar class.
> 
> Timothy Chen wrote:
> Sure, are you going to do that? I think it's easier to put it in a shared 
> place for now since it's going to take longer to merge to libprocess IMO.

Can i add that as a separate patch?


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---


On Oct. 12, 2015, 9:35 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 12, 2015, 9:35 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> fdb68b675582f603ffb3e96f31c1c9405e238567 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 12, 2015, 9:35 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

Added to cmake.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
  src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > 
> >
> > I thought you wanted to move this to somewhere shared? We can create a 
> > base puller class and move this there.
> 
> Jojy Varghese wrote:
> Thought about it a little more and realized that the functionality of 
> "untar a tarball into a dierctory" should belong in a common place like 
> libprocess. Its not a function of puller but maybe a Tar class.
> 
> Timothy Chen wrote:
> Sure, are you going to do that? I think it's easier to put it in a shared 
> place for now since it's going to take longer to merge to libprocess IMO.
> 
> Jojy Varghese wrote:
> Can i add that as a separate patch?

https://reviews.apache.org/r/39250


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---


On Oct. 12, 2015, 9:35 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 12, 2015, 9:35 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> fdb68b675582f603ffb3e96f31c1c9405e238567 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Alex Clemmer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review102161
---



src/Makefile.am (line 546)


Hey, could we consider adding a line like this to the `CMakeLists.txt` file 
in `src/`?


- Alex Clemmer


On Oct. 8, 2015, 2:37 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 8, 2015, 2:37 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> fdb68b675582f603ffb3e96f31c1c9405e238567 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 8, 2015, 2:37 a.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 5, 2015, 9:03 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

Added simple puller test.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
c2040b48ea43fdb29766994c244273d3fa9ee3cd 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > 
> >
> > I thought you wanted to move this to somewhere shared? We can create a 
> > base puller class and move this there.
> 
> Jojy Varghese wrote:
> Thought about it a little more and realized that the functionality of 
> "untar a tarball into a dierctory" should belong in a common place like 
> libprocess. Its not a function of puller but maybe a Tar class.

Sure, are you going to do that? I think it's easier to put it in a shared place 
for now since it's going to take longer to merge to libprocess IMO.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---


On Oct. 2, 2015, 5:18 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 2, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101339
---



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 143)


I think this is going to crash when user passes in a invalid sec value, 
since you check for error but you do a get in the if statement.

You should break it into two checks and do different actions.

Also remove the unnecessary parenthesis around timeoutSecs.isError()



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 179)


Align << on both lines.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 191)


Fix all the indentation with LOGs please.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 218)


You're missing the discard case here.
If timeout happens, you discard the future and I believe it will call the 
following callback here.

Therefore this is also going to crash if it times out.

I suggest we really write tests around these as I don't think this will 
work at all.


- Timothy Chen


On Oct. 2, 2015, 5:18 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 2, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 2, 2015, 5:18 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

addressed review comments.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
c2040b48ea43fdb29766994c244273d3fa9ee3cd 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101338
---



src/slave/containerizer/provisioner/docker/registry_client.cpp (line 171)


Fix the indentation here, we missed it last time.


- Timothy Chen


On Oct. 2, 2015, 5:18 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 2, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 2, 2015, 6:41 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
c2040b48ea43fdb29766994c244273d3fa9ee3cd 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 136)


Replace + with <<



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 173)


Fix identation, and let's just all use <<



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 186)


Does this warrent a LOG(INFO)? Perhaps VLOG(1)?
I suspect this is a pretty normal situation.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 189)


Don't need to assign this right? Just return  
_downloadTracker.at(layer.layerId).future()



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 209)


using namespace process and you can get rid of process:: here, we do that 
everywhere



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 227)


I thought you wanted to move this to somewhere shared? We can create a base 
puller class and move this there.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 245)


You need to return here.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 309)


This doesn't look right, it's simply a invalid timeout value that's passed 
to puller, nothing to do with remote name to canonial name



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 317)


Why break down layer timeout? Why not just one timeout?

And how is the user able to configure this pull timeout in the first place?

If we don't have give users the option to change this I suggest removing 
this from the interface and let's just use constants for now.


- Timothy Chen


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 317
> > 
> >
> > Why break down layer timeout? Why not just one timeout?
> > 
> > And how is the user able to configure this pull timeout in the first 
> > place?
> > 
> > If we don't have give users the option to change this I suggest 
> > removing this from the interface and let's just use constants for now.

The only reason being that user wont care about manifest vs pull timeout. They 
would care only about the total timeout. The split is so that we can cap their 
individual timeouts.

I havent added a flag for timeout and the interface uses Option so that if its 
not passed in, we use the default. We currently use constant default.


> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 209
> > 
> >
> > using namespace process and you can get rid of process:: here, we do 
> > that everywhere

I thought we are moving away from "using namespace process" ?


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 1, 2015, 6:40 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased with master


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101260
---



src/slave/containerizer/provisioner/docker/local_puller.hpp (line 54)


Do we use this or set this anywhere?
Can we remove this until we know what's the best way to set (or not) this?



src/slave/flags.cpp (line 118)


s/docker/Docker/g


- Timothy Chen


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Oct. 1, 2015, 6:40 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/local_puller.hpp, line 54
> > 
> >
> > Do we use this or set this anywhere?
> > Can we remove this until we know what's the best way to set (or not) 
> > this?

Also I think we are going to have a scheme to manage space and download sizes 
based on configured space and other poilcies, so I suggest leaving this out and 
leave a TODO around the whole size business.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101260
---


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 317
> > 
> >
> > Why break down layer timeout? Why not just one timeout?
> > 
> > And how is the user able to configure this pull timeout in the first 
> > place?
> > 
> > If we don't have give users the option to change this I suggest 
> > removing this from the interface and let's just use constants for now.
> 
> Jojy Varghese wrote:
> The only reason being that user wont care about manifest vs pull timeout. 
> They would care only about the total timeout. The split is so that we can cap 
> their individual timeouts.
> 
> I havent added a flag for timeout and the interface uses Option so that 
> if its not passed in, we use the default. We currently use constant default.

Can you also add the flag then? I think having a total timeout certainly makes 
sense, but also not sure what percentage really makes sense here.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > 
> >
> > I thought you wanted to move this to somewhere shared? We can create a 
> > base puller class and move this there.

Thought about it a little more and realized that the functionality of "untar a 
tarball into a dierctory" should belong in a common place like libprocess. Its 
not a function of puller but maybe a Tar class.


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101261
---


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

> On Oct. 1, 2015, 6:40 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 118
> > 
> >
> > s/docker/Docker/g

other flags use docker currently. Do you want only this one changed to Docker 
or all of them?


- Jojy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review101260
---


On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Oct. 1, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Oct. 1, 2015, 3:32 a.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

Integrated with store; review comments;


Repository: mesos


Description (updated)
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review100869
---



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 210)


Add defer


- Timothy Chen


On Sept. 28, 2015, 8:39 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Sept. 28, 2015, 8:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First iteration with basic functionality.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review100867
---



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 193)


We need to also insert into the tracker when we start a new download



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 196)


We don't need a dispatch here



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 218)


you can skip this whole .then and merge with the above onAny.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 245)


You can call onAny here, then you don't need to return anything.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 257)


kill the return



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 297)


We should return failure when timeout is <= 0 instead of just using the 
default and hide the bad input.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 313)


Space between foreach and (



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 331)


No need to return



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 338)


ditto


- Timothy Chen


On Sept. 28, 2015, 8:39 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Sept. 28, 2015, 8:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First iteration with basic functionality.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Sept. 25, 2015, 6:34 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

rebased with master


Repository: mesos


Description
---

First iteration with basic functionality.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Sept. 25, 2015, 11:14 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

fixed download order.


Repository: mesos


Description
---

First iteration with basic functionality.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

(Updated Sept. 22, 2015, 11:38 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

minor style changes.


Repository: mesos


Description
---

First iteration with basic functionality.


Diffs (updated)
-

  src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review99905
---


Patch looks great!

Reviews applied: [38443, 38579, 38580]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 11:31 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Sept. 21, 2015, 11:31 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First iteration with basic functionality.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
---

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

First iteration with basic functionality.


Diffs
-

  src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/38580/diff/


Testing
---

make check.


Thanks,

Jojy Varghese