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

2016-01-27 Thread Benjamin Bannier

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




src/master/main.cpp (line 112)


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)


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.

2016-01-27 Thread Alexander Rojas

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



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

2016-01-25 Thread Alexander Rojas

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