mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilbert Song" <songzihao1...@gmail.com>
Subject Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.
Date Sun, 20 Dec 2015 01:50:25 GMT


> On Dec. 18, 2015, 5:22 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/message.proto, line 72
> > <https://reviews.apache.org/r/40944/diff/5/?file=1157241#file1157241line72>
> >
> >     I think we should make this optional as well, since technically we don't really
require this.

Totally agreed.


> On Dec. 18, 2015, 5:22 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/spec.cpp, line 79
> > <https://reviews.apache.org/r/40944/diff/5/?file=1157243#file1157243line79>
> >
> >     Formatting seems a bit odd,
> >     Can you do:
> >     
> >     Try<JSON..> v1Compat = JSON::parse<JSON::Object>(
> >       stringify(manife....);

Sure. It looks better now, and it should be four spaces before `stringify` I am assuming.


> On Dec. 18, 2015, 5:22 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/spec.cpp, line 87
> > <https://reviews.apache.org/r/40944/diff/5/?file=1157243#file1157243line87>
> >
> >     Shouldn't this fit in the same line?

Unfortunately not. :(

We may still be able to make it look better. Here is my change:

    Try<docker::v2::ImageManifest::History::V1Compatibility> v1Compatibility =
      protobuf::parse<docker::v2::ImageManifest::History::V1Compatibility>(
          v1Compat.get());


- Gilbert


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


On Dec. 19, 2015, 5:43 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40944/
> -----------------------------------------------------------
> 
> (Updated Dec. 19, 2015, 5:43 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jojy Varghese, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixed protobuf parse failure when pulling a docker image.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250

>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75

>   src/slave/containerizer/mesos/provisioner/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2

>   src/tests/containerizer/provisioner_docker_tests.cpp 7c1197ed03df8b5d77d26307560535f9ff5bf350

> 
> Diff: https://reviews.apache.org/r/40944/diff/
> 
> 
> Testing
> -------
> 
> make check(ubuntu14.04 + clang3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


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