mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adam B <a...@mesosphere.io>
Subject Re: Review Request 49600: Added authz to /files/debug endpoint.
Date Tue, 05 Jul 2016 10:04:45 GMT

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



Looks good overall. I have just a few minor suggestions, and once you've addressed those and
Alexander's, it should be quick to commit.


src/common/http.cpp (line 74)
<https://reviews.apache.org/r/49600/#comment206096>

    Not yours, but s/EDNPOINTS/ENDPOINTS/ please



src/common/http.cpp (lines 77 - 78)
<https://reviews.apache.org/r/49600/#comment206106>

    Please also update `docs/authorization.md` and `docs/upgrades.md` to include `/files/debug`
(json endpoint unnecessary) in their lists of authorizable endpoints.



src/files/files.hpp (line 65)
<https://reviews.apache.org/r/49600/#comment206104>

    Shame we have to perpetuate these naked pointers. Any chance we could use a `shared_ptr`?



src/files/files.cpp (lines 87 - 88)
<https://reviews.apache.org/r/49600/#comment206097>

    Why did these have to move?



src/files/files.cpp (line 177)
<https://reviews.apache.org/r/49600/#comment206105>

    `/files/debug` specifically



src/files/files.cpp 
<https://reviews.apache.org/r/49600/#comment206098>

    Please add this back. Somebody put the blank line there to separate the deprecated .json
endpoints from the modern .json-less ones, and I (for one) appreciate the whitespace here.



src/tests/files_tests.cpp (lines 537 - 539)
<https://reviews.apache.org/r/49600/#comment206101>

    You're neither waiting on nor validating this `request`. Either do so, or remove it and
the FutureArg that sets it.


- Adam B


On July 4, 2016, 10:41 a.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49600/
> -----------------------------------------------------------
> 
> (Updated July 4, 2016, 10:41 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5369
>     https://issues.apache.org/jira/browse/MESOS-5369
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added authz to /files/debug endpoint.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
>   src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49600/diff/
> 
> 
> Testing
> -------
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.DebugTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>


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