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 48014: Added the route handler for v1 agent API.
Date Mon, 30 May 2016 01:21:41 GMT


> On May 29, 2016, 8:14 p.m., Benjamin Hindman wrote:
> > src/slave/http.cpp, line 265
> > <https://reviews.apache.org/r/48014/diff/1/?file=1400619#file1400619line265>
> >
> >     Unused variable, but I'm guessing it's used in a subsequent patch. One thought
here, to avoid the declared but undefined `response` variable here and in the master/http.cpp,
would be to define a helper function that you used instead? Not a blocker IMHO, but wanted
to suggest something like:
> >     
> >     ```
> >     // Short comment about what this is doing here ...
> >     auto serializer = [request](const v1::agent::Response& response) -> Response
{
> >       ContentType responseContentType;
> >       if (request.acceptsMediaType(APPLICATION_JSON)) {
> >         responseContentType = ContentType::JSON;
> >       } else if (request.acceptsMediaType(APPLICATION_PROTOBUF)) {
> >         responseContentType = ContentType::PROTOBUF;
> >       } else {
> >         return NotAcceptable(
> >             string("Expecting 'Accept' to allow ") +
> >             "'" + APPLICATION_PROTOBUF + "' or '" + APPLICATION_JSON + "'");
> >       }
> >     
> >       // TODO(vinod): Support JSONP requests?
> >       return OK(serialize(responseContentType, response),
> >                 stringify(responseContentType));
> >     };
> >     ```
> >     
> >     Then in the code do things like:
> >     
> >     ```
> >     switch (call.type()) {
> >       ...
> >       case v1::agent::Call::GET_FLAGS:
> >         return serializer(evolve<v1::master::Response::GET_FLAGS>(_flags()));
> >       ...
> >       case v1::agent::Call::SOMETHING_ASYNC:
> >         return functionReturningFuture()
> >           .then([](const v1::agent::Response& response) {
> >             return serializer(response);
> >           });
> >           
> >       case v1::agent::Call::SOMETHING_ASYNC:
> >         return functionReturningFuture()
> >           .then(serializer); // This might even work?
> >     }
> >     ```

done. thanks.


- Vinod


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


On May 30, 2016, 1:21 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48014/
> -----------------------------------------------------------
> 
> (Updated May 30, 2016, 1:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2720
>     https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is just the basic structure for handling the calls. Actual calls
> will be implemented later.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 64a99d539a0957b92d42c25c8dda298199383018 
>   src/slave/slave.hpp 78a6ecfe29881f1723d7d603a99962afdd5a81a8 
>   src/slave/slave.cpp 97d15da2a20c74d3a502abac68cc7a695e06a189 
>   src/slave/validation.hpp 070a77115f0d6f22615fc5bd42537e6d1b86f0bf 
>   src/slave/validation.cpp dccc788d7ec782b08abe4b58d8b92031939fa2d7 
> 
> Diff: https://reviews.apache.org/r/48014/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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