You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2016/01/25 13:57:58 UTC

Re: Review Request 42648: Moved http authenticator initialization to main.

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

(Updated Jan. 25, 2016, 1:57 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan Schlicht, and Till Toenshoff.


Summary (updated)
-----------------

Moved http authenticator initialization to main.


Repository: mesos


Description (updated)
-------

Moves the code which initializes instances of `process::http::authentication::Authenticator` from `mesos::internal::Master` to the master main file.

The change is needed in order to show that libprocess http authentication is turned at the system process level and not at the libprocess process level.


Diffs (updated)
-----

  src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 

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


Testing (updated)
-------

make check

manual tests.


Thanks,

Alexander Rojas


Re: Review Request 42648: Moved http authenticator initialization to main.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42648/#review116648
-----------------------------------------------------------




src/master/main.cpp (line 112)
<https://reviews.apache.org/r/42648/#comment177714>

    Personally I find this adds more confusion. There are both `process::http::authentication` and `mesos::http::authentication`, and here we introduce an alias for only of of them (and (sadly) we don't seem to be too fond of namespace aliases anyway). I personally would prefer to drop this here and just going full verbose (since we cannot use `auto`).



src/tests/cluster.cpp (lines 213 - 245)
<https://reviews.apache.org/r/42648/#comment177717>

    Could this be factored out into a separate function we'd call from both `main` and here? The `EXIT` certainly wouldn't make this a nice function to call, but this is a lot of repeated, not 100% trivial code ...


- Benjamin Bannier


On Jan. 27, 2016, 10:23 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42648/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moves the code which initializes instances of `process::http::authentication::Authenticator` from `mesos::internal::Master` to the master main file.
> 
> The change is needed in order to show that libprocess http authentication is turned at the system process level and not at the libprocess process level.
> 
> It also adds the same initialization as in main to the test function `StartMaster`.
> 
> 
> Diffs
> -----
> 
>   src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
> 
> Diff: https://reviews.apache.org/r/42648/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 42648: Moved http authenticator initialization to main.

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42648/
-----------------------------------------------------------

(Updated Jan. 27, 2016, 10:23 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan Schlicht, and Till Toenshoff.


Changes
-------

Rebase.


Repository: mesos


Description
-------

Moves the code which initializes instances of `process::http::authentication::Authenticator` from `mesos::internal::Master` to the master main file.

The change is needed in order to show that libprocess http authentication is turned at the system process level and not at the libprocess process level.

It also adds the same initialization as in main to the test function `StartMaster`.


Diffs (updated)
-----

  src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 

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


Testing
-------

make check

manual tests.


Thanks,

Alexander Rojas


Re: Review Request 42648: Moved http authenticator initialization to main.

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42648/
-----------------------------------------------------------

(Updated Jan. 27, 2016, 7:50 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan Schlicht, and Till Toenshoff.


Changes
-------

Actually removes functionality from `mesos::Master`


Repository: mesos


Description (updated)
-------

Moves the code which initializes instances of `process::http::authentication::Authenticator` from `mesos::internal::Master` to the master main file.

The change is needed in order to show that libprocess http authentication is turned at the system process level and not at the libprocess process level.

It also adds the same initialization as in main to the test function `StartMaster`.


Diffs (updated)
-----

  src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 

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


Testing
-------

make check

manual tests.


Thanks,

Alexander Rojas