> On July 21, 2017, 1:43 a.m., Qian Zhang wrote: > > include/mesos/mesos.proto > > Line 2843 (original), 2922 (patched) > > <https://reviews.apache.org/r/60932/diff/2-3/?file=1780157#file1780157line2922> > > > > Why is `value` an optional field? I think there have to a value for any > > possible semantics, right? > > Gilbert Song wrote: > The same, I don't think an `optional` field hurts here and it makes it > more flexible. Let's chat tomorrow. > > Qian Zhang wrote: > Sure, let's chat on it. And can we have an expert on this area (BenM or > Jie) to provide some comments? > > Jie Yu wrote: > for this, i think we can make it required. we can always change it to > optional in the future.
I think we should avoid using required fields, because we don't deal with errors at the application level, right now the messages are dropped at the parsing layers when required fields are missing. We could update our parsing layers to use the "partial" parsing that protobuf supplies [[1]](https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message#heavy-io) to ignore missing required fields, but we then have to update our application level code to treat the field as optional anyway in terms of validating it based on field presence. Also, I'm not sure we can assume the use of partial parsing outside of mesos. So, it seems to me we might as well try to stick with optional and perform application level validation of missing fields (rather than let the parsing fail). i.e. `optional uint64 value = 2; // Required.` Thoughts? [1] https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message#heavy-io - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60932/#review181087 ----------------------------------------------------------- On July 20, 2017, 11:57 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60932/ > ----------------------------------------------------------- > > (Updated July 20, 2017, 11:57 p.m.) > > > Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and > Zhitao Li. > > > Bugs: MESOS-6162 > https://issues.apache.org/jira/browse/MESOS-6162 > > > Repository: mesos > > > Description > ------- > > Only statistics information for blkio in protobuf. > > > Diffs > ----- > > include/mesos/mesos.proto 8f8079bd7c2de4e8b2f8f9a56e2731b77b8e1575 > include/mesos/v1/mesos.proto 720f307f8d738b0787e7c47be7ee15be38b2c0d0 > > > Diff: https://reviews.apache.org/r/60932/diff/3/ > > > Testing > ------- > > make > > > Thanks, > > Gilbert Song > >
