You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jojy Varghese <jo...@mesosphere.io> on 2015/10/01 05:32:18 UTC

Re: Review Request 38580: Added docker registry RemotePuller

-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 317
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line317>
> >
> >     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
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line209>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line227>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 317
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line317>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line227>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line227>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Oct. 1, 2015, 6:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/remote_puller.cpp, line 227
> > <https://reviews.apache.org/r/38580/diff/7/?file=1088724#file1088724line227>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment158634>

    Replace + with <<



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 173)
<https://reviews.apache.org/r/38580/#comment158635>

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



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 186)
<https://reviews.apache.org/r/38580/#comment158636>

    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)
<https://reviews.apache.org/r/38580/#comment158637>

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



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 209)
<https://reviews.apache.org/r/38580/#comment158638>

    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)
<https://reviews.apache.org/r/38580/#comment158639>

    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)
<https://reviews.apache.org/r/38580/#comment158640>

    You need to return here.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 309)
<https://reviews.apache.org/r/38580/#comment158641>

    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)
<https://reviews.apache.org/r/38580/#comment158644>

    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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment158699>

    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)
<https://reviews.apache.org/r/38580/#comment158700>

    Align << on both lines.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 191)
<https://reviews.apache.org/r/38580/#comment158701>

    Fix all the indentation with LOGs please.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 218)
<https://reviews.apache.org/r/38580/#comment158703>

    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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment158698>

    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

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review102161
-----------------------------------------------------------



src/Makefile.am (line 546)
<https://reviews.apache.org/r/38580/#comment159721>

    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

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 236
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line236>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 129
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line129>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 295
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line295>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 373
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line373>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line51>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line51>
> >
> >     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
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line129>
> >
> >     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
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line133>
> >
> >     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
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line236>
> >
> >     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
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line295>
> >
> >     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
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line373>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105134
-----------------------------------------------------------



src/Makefile.am (line 564)
<https://reviews.apache.org/r/38580/#comment163530>

    We use tab indentation in the Makefilm.am



src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp (line 57)
<https://reviews.apache.org/r/38580/#comment163540>

    I think this is redudant.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp (line 69)
<https://reviews.apache.org/r/38580/#comment163541>

    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)
<https://reviews.apache.org/r/38580/#comment163542>

    We don't use alias namespaces tpyically.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 51)
<https://reviews.apache.org/r/38580/#comment163543>

    Didn''t you pull these out?



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 129)
<https://reviews.apache.org/r/38580/#comment163544>

    Move the timeout after into the RemotePullerProcess.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 133)
<https://reviews.apache.org/r/38580/#comment163546>

    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)
<https://reviews.apache.org/r/38580/#comment163548>

    Remove extra space



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 216)
<https://reviews.apache.org/r/38580/#comment163547>

    Fix indentation



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 236)
<https://reviews.apache.org/r/38580/#comment163553>

    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)
<https://reviews.apache.org/r/38580/#comment163555>

    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)
<https://reviews.apache.org/r/38580/#comment163556>

    Can you rebase on all your refactoring patches?



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 373)
<https://reviews.apache.org/r/38580/#comment163558>

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



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 380)
<https://reviews.apache.org/r/38580/#comment163559>

    Remove extra space between ' and :



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 385)
<https://reviews.apache.org/r/38580/#comment163560>

    ditto



src/slave/flags.cpp (line 118)
<https://reviews.apache.org/r/38580/#comment163537>

    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)
<https://reviews.apache.org/r/38580/#comment163538>

    How about "Default Docker registry server host"



src/slave/flags.cpp (line 128)
<https://reviews.apache.org/r/38580/#comment163539>

    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 
>   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

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 4, 2015, 10:05 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 43
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line43>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment163564>

    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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment164128>

    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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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.


Changes
-------

review


Repository: mesos


Description
-------

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-----

  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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Nov. 7, 2015, 9:36 a.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51
> > <https://reviews.apache.org/r/38580/diff/24/?file=1118727#file1118727line51>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Nov. 7, 2015, 9:36 a.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 246
> > <https://reviews.apache.org/r/38580/diff/24/?file=1118727#file1118727line246>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105579
-----------------------------------------------------------



src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp (line 45)
<https://reviews.apache.org/r/38580/#comment164189>

    We pull multiple images with each puller, I would clarify that "Pulls docker images from docker registry"



src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp (line 47)
<https://reviews.apache.org/r/38580/#comment164190>

    We had quite some feeedback about not really knowing what does Remote mean in RemotePuller. How about let's rename this to RegistryPuller instead?
    
    I think this matches what it does more closely, since a Registry could technically even live in the same box.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 51)
<https://reviews.apache.org/r/38580/#comment164191>

    This is no longer true since we pulled these out of RemotePuller right? Same as all the other ones.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 131)
<https://reviews.apache.org/r/38580/#comment164198>

    Move this into RegistryPullerProcess, introduce a new method inside to call after with.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 246)
<https://reviews.apache.org/r/38580/#comment164192>

    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.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 365)
<https://reviews.apache.org/r/38580/#comment164193>

    I would delete onDiscard and onFailed here.
    Not sure if moving this to the top makes any difference.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 376)
<https://reviews.apache.org/r/38580/#comment164194>

    you don't need process::defer here? You skipped the namespace here but used it in Line 226



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 381)
<https://reviews.apache.org/r/38580/#comment164195>

    Fix the indentation as we discussed earlier,
    
    foreach (const .......,
             manifest.fsLayerInfos) {
             
    }



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 391)
<https://reviews.apache.org/r/38580/#comment164196>

    Move these before the return line.



src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp (line 394)
<https://reviews.apache.org/r/38580/#comment164197>

    Can we move these callbacks into smaller named methods? 
    
    I think it makes the flow much clearer:
    
    return registryClient->getManifest()
        .then(.... return downloadLayers()
        .then(.... return untarLayers())


- Timothy Chen


On Nov. 7, 2015, 12:11 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Integrated remote puller with store. Tests will follow.
> 
> 
> Diffs
> -----
> 
>   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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/#review105325
-----------------------------------------------------------



src/tests/containerizer/provisioner_docker_tests.cpp (line 1191)
<https://reviews.apache.org/r/38580/#comment163853>

    I dont think this API of read does the non-block. In fact i think it expects it and errors out if the fd passed in is blocking.



src/tests/containerizer/provisioner_docker_tests.cpp (line 1217)
<https://reviews.apache.org/r/38580/#comment163851>

    Copying the tar buffer into output buffer before sending it out. We are simulating a blob layer sent from server.


- Jojy Varghese


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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment163824>

    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)
<https://reviews.apache.org/r/38580/#comment163823>

    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)
<https://reviews.apache.org/r/38580/#comment163825>

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



src/tests/containerizer/provisioner_docker_tests.cpp (line 1217)
<https://reviews.apache.org/r/38580/#comment163826>

    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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment163587>

    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

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 4, 2015, 10:54 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 373
> > <https://reviews.apache.org/r/38580/diff/19/?file=1115760#file1115760line373>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment163586>

    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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38580/
-----------------------------------------------------------

(Updated Oct. 6, 2015, 4:44 p.m.)


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


Changes
-------

Fixed test.


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 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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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

Posted by Timothy Chen <tn...@apache.org>.

> On Oct. 1, 2015, 6:40 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/local_puller.hpp, line 54
> > <https://reviews.apache.org/r/38580/diff/6/?file=1088226#file1088226line54>
> >
> >     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

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Oct. 1, 2015, 6:40 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 118
> > <https://reviews.apache.org/r/38580/diff/6/?file=1088234#file1088234line118>
> >
> >     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

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38580/#comment158633>

    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)
<https://reviews.apache.org/r/38580/#comment158632>

    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

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
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