mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anand Mazumdar <an...@apache.org>
Subject Re: Review Request 48705: Implemented LIST_FILES Call in v1 agent API.
Date Fri, 24 Jun 2016 02:15:10 GMT


> On June 24, 2016, 12:51 a.m., Anand Mazumdar wrote:
> > src/slave/http.cpp, lines 666-714
> > <https://reviews.apache.org/r/48705/diff/3/?file=1423602#file1423602line666>
> >
> >     We can use the `FilesError` type's to construct the response here and just have
one public `browse` that does both `resolve()`/`authorize()`.
> 
> zhou xing wrote:
>     Hi All,
>     
>     I'm thinking whether it is possible that we add a new method named browseFiles in
FilesProcess like:
>     
>       Future<Response> browseFiles(
>           const mesos::master::Call& call,
>           const Option<string>& principal,
>           ContentType contentType);
>     
>     This method has similar function as:
>     
>       Future<Response> browse(
>           const Request& request,
>           const Option<string>& principal);
>      
>      The only difference between them is where and how to extract the params. If this
is possible, that would be easier for coding and sharing public logic code?

hmm, this would have been certainly the easier approach but not the correct one. If you have
a look at `Files`, it hardly makes any sense for it's public methods to return a `http::Response`
directly. None of the actors anywhere in our code directly return a `http::Response` unless
they are set up via the `route` handler or are helper methods called by the main handler.

`FilesProcess` in `browse()` currently returns a `http::Response` directly since it is set
up via the `route` handler. Going forward, we would eventually deprecate the `files/*` endpoints
and just make the Master/Agent call into the `Files` interface directly. 

We should be following a similar approach for the `READ_FILES` implementation.


- Anand


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


On June 19, 2016, 8:38 p.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48705/
> -----------------------------------------------------------
> 
> (Updated June 19, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5514
>     https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented LIST_FILES Call in v1 agent API.
> 
> 
> Diffs
> -----
> 
>   include/mesos/v1/agent/agent.proto 5ac6d4c93a826f0e7a3654d7dcc02b16424f90ce 
>   src/slave/http.cpp 77834111bf51b58ba5e12f262b725202f7d3a7bf 
>   src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 
> 
> Diff: https://reviews.apache.org/r/48705/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>


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