mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Chen" <tnac...@apache.org>
Subject Re: Review Request 38580: Added docker registry RemotePuller
Date Wed, 04 Nov 2015 22:37:53 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message