> On May 10, 2016, 5:41 p.m., Kevin Klues wrote:
> > include/mesos/docker/v1.proto, lines 55-60
> > <https://reviews.apache.org/r/47198/diff/1/?file=1378943#file1378943line55>
> >
> >     I don't quite understand this comment.  WHy isn't this just a repeated 
> > optional string?
> 
> Gilbert Song wrote:
>     WHy isn't this just a repeated optional string?
>     I thought about it and did plan to use repeated `Label`, but in our first 
> class mesos.proto, all labels are used as an optional Labels since a repeated 
> Label is included. And the reason it is not a string is that any metadata 
> label in docker image should be a key-value pair (that is identical to how we 
> defind Label in mesos.proto.
>     
>     About the commnet:
>     Since we use protobuf::parse() to automatically parse all JSON to 
> protobuf message, the parse function relies on the field name and they are 
> case sensitive. It means we cannot use `Labels` (which is identical to docker 
> manifest), otherwise protobuf::parse would return an error. You can see all 
> other field names are exactly identical to those names in docker image 
> manifest execpt this one (`labels`).
> 
> Kevin Klues wrote:
>     It just feels weird to me to have this file depend on mesos.proto. If we 
> don't want pure strings, I would expect to duplicate something similar to the 
> `Labels` message in mesos.proto, but not pull it in directly. Curious what 
> others think.

Hmm.., it does look strange to have this dependency. Seems like we do not have 
duplicate proto msg in code base. If it duplicates the Labels in mesos.proto, 
will they be capatible? Or will they confuse people.


- Gilbert


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


On May 10, 2016, 4:21 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47198/
> -----------------------------------------------------------
> 
> (Updated May 10, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Jie Yu, and Kevin 
> Klues.
> 
> 
> Bugs: MESOS-5272
>     https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added labels to docker v1 spec config.
> 
> 
> Diffs
> -----
> 
>   include/mesos/docker/v1.proto ff18f8cadfce3315467f194d50d3469fa839f686 
> 
> Diff: https://reviews.apache.org/r/47198/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Reply via email to