mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 48008: Added v1 protos for master and agent APIs.
Date Sun, 29 May 2016 21:43:24 GMT


> On May 29, 2016, 7:12 p.m., Anand Mazumdar wrote:
> > include/mesos/v1/master.proto, line 59
> > <https://reviews.apache.org/r/48008/diff/4/?file=1400695#file1400695line59>
> >
> >     s/streaming//
> >     
> >     hmm.. I find it a bit odd to include the business logic of the application itself
when definind the request/response schema.

wanted to call it out because everything else returns a Response (as described in the comment
on top of the enum). streaming response is no different from synchronous response as far as
comments in this file are concerned. if this was a grpc proto file stream would be specified
here.


> On May 29, 2016, 7:12 p.m., Anand Mazumdar wrote:
> > include/mesos/v1/agent.proto, lines 37-48
> > <https://reviews.apache.org/r/48008/diff/4/?file=1400693#file1400693line37>
> >
> >     For readability, would it be a good idea to have these `Type`'s sorted in alphabetical
order and have `GET_` being grouped with their `SET_` types?
> >     
> >     Here is how I imagine them to look with `UNKNOWN` being the first since we can't
do anything about it:
> >     
> >     ```
> >     `GET_FLAGS` = 2;
> >     `GET_LOGGING_LEVEL` = 3;
> >     `SET_LOGGING_LEVEL` = 4;
> >     `GET_METRICS` = 5;
> >     and so on..
> >     ```
> >     
> >     The same ordering should be done for the corresponding `Call::Foo` messages
below. What do you think?

as discussed offine, i'll put spaces between functional groups of calls for redability.


- Vinod


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


On May 29, 2016, 9:38 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48008/
> -----------------------------------------------------------
> 
> (Updated May 29, 2016, 9:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2257 and MESOS-2720
>     https://issues.apache.org/jira/browse/MESOS-2257
>     https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Also added common protos to v1/mesos.proto.
> 
> 
> Diffs
> -----
> 
>   include/mesos/v1/agent.hpp PRE-CREATION 
>   include/mesos/v1/agent.proto PRE-CREATION 
>   include/mesos/v1/master.hpp PRE-CREATION 
>   include/mesos/v1/master.proto PRE-CREATION 
>   include/mesos/v1/mesos.proto af95b5233158c68db356a4c178d9811cf7bf665d 
> 
> Diff: https://reviews.apache.org/r/48008/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message