> On June 5, 2015, 6:42 p.m., Vinod Kone wrote:
> > I made some minor comments below but I think a better way to do this is to 
> > *not* write custom masterinfo json <-> protobuf converters. I prefer we 
> > just add a new optional field (say ipAddress of type string). Then you can 
> > just leverage the already existing generic protobuf <-> json converters.
> 
> Niklas Nielsen wrote:
>     We have been working on this for a while now and we are using 
> JSON::Protobuf() aldready and can enhance it further with your suggestion. 
> However, the current approach isn't semantically different from the one you 
> suggest and we would like to move forward and make that change later. Is that 
> OK with you?

Hey Vinod - as Niklas pointed out, we have invested a significant amount of 
time on this one, including the manual testing I've done (as summarized on 
MESOS-2304) and I'd be really reluctant to start again from scratch.

As I don't really think there is any semantic difference; my approach does not 
introduce any performance penalty (in fact, I believe it may even be better 
than a 'generic' one); and, finally, that this does not impact the readability 
of the code, we should commit the code 'as is' (with your suggestions below) 
and move on.

There are a ton of features and work to do, and I'd like us to focus on 
important issues.


- Marco


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


On June 3, 2015, 9 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34687/
> -----------------------------------------------------------
> 
> (Updated June 3, 2015, 9 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2340
>     https://issues.apache.org/jira/browse/MESOS-2340
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-2340
> 
> This is a preliminary step to enabling JSON API
> for Master Discovery via Zookeeper.
> 
> We currently save the MasterInfo PB to ZK
> serializing directly the binary data, so that
> for clients to retrieve that information, they
> need to either link up with libmesos or
> obtain the PB definition (in mesos/mesos.proto).
> 
> This change only provides the (de)serialization
> utility methods and associated tests.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
>   src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a 
>   src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
>   src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 
>   src/tests/common/parse_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34687/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Reply via email to