> On Dec. 8, 2015, 8:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/process.hpp, lines 65-86
> > <https://reviews.apache.org/r/38000/diff/19/?file=1155107#file1155107line65>
> >
> >     Why isn't this in the http header? It looks like firewall belongs there 
> > too since it's more aptly namespaced as 'http::Firewall'?

I will move the firewall functions in a different request.


> On Dec. 8, 2015, 8:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1205-1223
> > <https://reviews.apache.org/r/38000/diff/21/?file=1155956#file1155956line1205>
> >
> >     Can you instead just add another handler to the existing HttpProcess 
> > above, rather than introducing another Process here?
> >     
> >     There is already a handler for 'auth', so we can add one called 
> > '/authenticated' and document why '/auth' is still there (looks like it is 
> > going to be obviated by this?).

About this one, there is one test that checks authentication which is handled 
through the auth method. That test (and its functionality) become obsolete with 
this patch, should I just removed?


> On Dec. 8, 2015, 8:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2400-2402
> > <https://reviews.apache.org/r/38000/diff/21/?file=1155955#file1155955line2400>
> >
> >     Why are you dispatching here rather than calling directly? Note though 
> > that I'm not sure we should keep `schedule`.

Mostly because BenH has strongly discourage me from calling methods directly 
from processes. Moreover, once the cleaning of unused sequences is implemented, 
is imposible to guarantee that requests are being done from the same thread 
always, so we may have concurrency issues.


- Alexander


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


On Dec. 8, 2015, 3:38 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38000/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 3:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3233
>     https://issues.apache.org/jira/browse/MESOS-3233
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds functions which allow libprocess users to register HTTP authenticators.
> Overloads `ProcesBase::route()` to allow for registering of authenticating 
> endpoints.
> Includes tests.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/event.hpp 
> a03824c061c4a0eb865b163999a763635e56744c 
>   3rdparty/libprocess/include/process/process.hpp 
> 81c094414d4d5ac5eb593df2a6d14aaacb19a826 
>   3rdparty/libprocess/src/authentication_router.hpp 
> 5777deafd7420324627802a7ab9da9aaa2b46825 
>   3rdparty/libprocess/src/process.cpp 
> e93709d3bb4ac588457bb9331fc05ec5ab539f6d 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38000/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to