> On May 11, 2016, 12:41 a.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`).
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. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47198/#review132575 ----------------------------------------------------------- On May 10, 2016, 11: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, 11: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 > >
