> On June 10, 2015, 8:37 p.m., Ben Mahler wrote:
> > How about adding an 'Address' message, which can contain 'ip' and 'port' 
> > for now?
> > 
> > ```
> > message Address {
> >   required string ip;
> >   required uint32 port;
> >   
> >   // Later we can add 'hostname' or 'public_hostname', etc!
> > }
> > ```
> > 
> > In the future, we can further use this in other protobufs to have a 
> > conistent representation (e.g. SlaveInfo only has 'hostname' and 'port' 
> > currently, no 'ip'!). That way, you can add an address to MasterInfo:
> > 
> > ```
> > message MasterInfo {
> >   required string id = 1;
> >   required uint32 ip = 2;
> >   required uint32 port = 3 [default = 5050];
> >   optional string pid = 4;
> >   optional string hostname = 5;
> >   
> >   optional Address address = 6;
> > }
> > ```
> > 
> > This way, you don't need all the custom logic introduced here, and 
> > consequently compatibility is easier to test (i.e. we have already tested 
> > our JSON <-> Protobuf conversion facilities).
> > 
> > Have you guys considered this approach?

@BenH, Vinod and BenM: We have too many chefs her. Let's please assign one to 
drive this. It sounds like we have to take this back to the design (and JIRA 
discussions).

Here is my concern; I was assigned as shepherd on this and got blocked in the 
11th hour.
There is very little space to iterate on this and the patch would have worked 
fine. It was a part of a bigger patch set (zookeeper detection for HTTP, which 
Vinod drove) and I am amazed that a conversion between proto to json and 
vice-versa takes more than 2 weeks back and forth, to then (most likely) 
getting dropped. This is one example where both committers and contributors 
(us) have a bad experience. I would like to know how to get better and avoid 
this in the future, without giving up 'shepherd'ship.

@Marco; I am sorry that we wasted your (and our) time. Let's try to fix the 
process. It hurts me as well as I thought we had this in a good shape but it 
sounds to me like we need to drop this patch.


- Niklas


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


On June 5, 2015, 1:18 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34687/
> -----------------------------------------------------------
> 
> (Updated June 5, 2015, 1:18 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2807
>     https://issues.apache.org/jira/browse/MESOS-2807
> 
> 
> 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