mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 52349: Add protobuf messages for OCI image spec.
Date Tue, 03 Jan 2017 13:53:28 GMT


> On Dec. 16, 2016, 8:32 a.m., Jie Yu wrote:
> > include/mesos/oci/spec.proto, line 90
> > <https://reviews.apache.org/r/52349/diff/6/?file=1581544#file1581544line90>
> >
> >     How do we support parsing `annotations`?
> >     
> >     Should we use a different name so we can do manual parsing?
> >     
> >     Also, i'd like to introduce a common `Label` message instead of `EntryWithStringValue`
> 
> Qian Zhang wrote:
>     > How do we support parsing `annotations`?
>     
>     That's the suggested way to handle `map` in protobuf, see https://developers.google.com/protocol-buffers/docs/proto#maps
(the backwards compatible way) for details, I think `annotations` will be automatically parsed
in this way.
>     
>     > Also, i'd like to introduce a common `Label` message instead of `EntryWithStringValue`
>     
>     Do you mean we rename `EntryWithStringValue` to `Label` and define it in a common
place rather than in this file?
> 
> Jie Yu wrote:
>     Ah, didn't realize that Map is already supported in proto2. We definitely need to
add parsing support for that (i.e., json -> protobuf). Not sure how backwards compatibility
will be handled. However, if it is used in agent only, it should not be a problem.
> 
> Qian Zhang wrote:
>     The protobuf that we are using (protobuf-2.6.1) does not support `map`, as MPark
mentioned in the dev list, we need to upgrade to protobuf 3.0.0 to have `map` support. Do
you think we need to do the upgrade right away, or we can use the backwards compatible way
mentioned in the above link first in this project and do the upgrade later?

Let's follow the similar way of https://reviews.apache.org/r/47199/ to do the manual parsing
for now, and in future when we upgrade the protobuf to a newer version in Mesos to support
`Map`, we can remove that manual parsing logic and just use the `Map` keyword.


- Qian


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


On Jan. 3, 2017, 9:53 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52349/
> -----------------------------------------------------------
> 
> (Updated Jan. 3, 2017, 9:53 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add protobuf messages for OCI image spec.
> 
> 
> Diffs
> -----
> 
>   include/mesos/oci/spec.hpp PRE-CREATION 
>   include/mesos/oci/spec.proto PRE-CREATION 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
> 
> Diff: https://reviews.apache.org/r/52349/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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