Re: Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[Anand Mazumdar]

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



hmm, we can do some refactoring here. Have a look at my later comments on how 
we can go about it:


src/files/files.hpp (lines 77 - 85)


Instead of exposing these public functions, we should define a single 
function `browse` that handles authorization/resolve in one place i.e. 
something like:

```cpp
Future> browse(
const string& path);
```

^^ Let's go over each of these new types in detail. `FilesError` is a type 
deriving from `Error` that the master/agent can use to construct the response 
i.e. if the type is `NOT_FOUND`, they can construct a `404` response etc. We 
would need similar error types when authorization fails etc.

```cpp
class FilesError : public Error
{
public:
  enum Type
  {
NOT_FOUND;

  }
  
  explicit FilesError(Type _type)
: Error(stringify(type)), type(_type) {}
  
  Type type;
};
```

`FileInfo` is a new protobuf that we would define that has information 
about the file itself. It would have information that `jsonFileInfo` returns 
i.e. would have the same fields. This is how the new `ListFiles` response would 
look like:

```cpp
message ListFiles {
  repeated FileInfo file_infos = 1;
}
```



src/files/files.cpp (lines 119 - 121)


hmmm, we would like `_browse`'s return type to be something like:

```cpp
Try _browse(...)
```

The old API's `browse` would then construct a `JSON::Array` from `FileInfo` 
via `model(...)` function that we would define in `src/common/http.cpp`.


- Anand Mazumdar


On June 14, 2016, 7:19 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48704/
> ---
> 
> (Updated June 14, 2016, 7:19 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
> ---
> 
> Updated FilesProcess to support List_Files Call in Operator API v1.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 
> 
> Diff: https://reviews.apache.org/r/48704/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.BrowseTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[haosdent huang]

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



LGTM ;)

- haosdent huang


On June 14, 2016, 7:19 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48704/
> ---
> 
> (Updated June 14, 2016, 7:19 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
> ---
> 
> Updated FilesProcess to support List_Files Call in Operator API v1.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 
> 
> Diff: https://reviews.apache.org/r/48704/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.BrowseTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[haosdent huang]

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




src/files/files.hpp (line 83)


I think we may consider create a extra function only return file list. So 
that we no need to parse `JSON::Array` in following patches.


- haosdent huang


On June 14, 2016, 7:19 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48704/
> ---
> 
> (Updated June 14, 2016, 7:19 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
> ---
> 
> Updated FilesProcess to support List_Files Call in Operator API v1.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 
> 
> Diff: https://reviews.apache.org/r/48704/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.BrowseTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[Abhishek Dasgupta]

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

(Updated June 14, 2016, 7:19 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
---

Updated FilesProcess to support List_Files Call in Operator API v1.


Diffs
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 

Diff: https://reviews.apache.org/r/48704/diff/


Testing (updated)
---

Ubuntu 16.04:

sudo GTEST_FILTER="FilesTest.BrowseTest" make -j4 check


Thanks,

Abhishek Dasgupta

Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[Abhishek Dasgupta]

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

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
---

Updated FilesProcess to support List_Files Call in Operator API v1.


Diffs
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 

Diff: https://reviews.apache.org/r/48704/diff/


Testing
---

Ubuntu 16.04:

sudo GTEST_FILTER="*AgentAPITest.ListFiles*" make -j4 check


Thanks,

Abhishek Dasgupta