Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-19 Thread Greg Mann


> On Jan. 20, 2016, 12:44 a.m., Michael Park wrote:
> > src/tests/reservation_endpoints_tests.cpp, lines 1173-1184
> > 
> >
> > Why are we testing for this?

When we met previously, Vinod mentioned that we sometimes equate 
default-initialized values with null values in the codebase, so I though it 
worth testing that case here.


> On Jan. 20, 2016, 12:44 a.m., Michael Park wrote:
> > src/tests/reservation_endpoints_tests.cpp, lines 1200-1201
> > 
> >
> > It seems like this framework is being spun up just to reserve resources 
> > on this agent. Do we not have a way to reserve the resources via the HTTP 
> > endpoint?

Unfortunately not; currently, the /reserve and /unreserve endpoints do not work 
when HTTP authentication is disabled, as is the case in this test.


- Greg


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


On Jan. 20, 2016, 12:11 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> ---
> 
> (Updated Jan. 20, 2016, 12:11 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added reservation endpoint test without auth and principal.
> 
> Currently, dynamic reservation endpoints will not work when HTTP 
> authentication is not set. This test checks that these endpoints will fail as 
> expected in this case.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42368/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, 
> was added, and make check was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-19 Thread Greg Mann

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

(Updated Jan. 20, 2016, 1:30 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Changes
---

Addressed comments.


Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195


Repository: mesos


Description
---

Added reservation endpoint test without auth and principal.

Currently, dynamic reservation endpoints will not work when HTTP authentication 
is not set. This test checks that these endpoints will fail as expected in this 
case.


Diffs (updated)
-

  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 

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


Testing
---

A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was 
added, and make check was used to test. The new test was also run with 
`--gtest_repeat=1000 --gtest_break_on_failure=1`.


Thanks,

Greg Mann



Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-19 Thread Michael Park

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



src/tests/reservation_endpoints_tests.cpp (lines 1156 - 1157)


Could you update `createReservationInfo` to take `Option` and 
use `createReservationInfo()` to get an empty `ReservationInfo`?



src/tests/reservation_endpoints_tests.cpp (lines 1160 - 1162)


Can we not continue to overwrite this? It's difficult to keep track of the 
changing states.



src/tests/reservation_endpoints_tests.cpp (lines 1173 - 1184)


Why are we testing for this?



src/tests/reservation_endpoints_tests.cpp (lines 1186 - 1198)


Could you explain why this is expected to fail?



src/tests/reservation_endpoints_tests.cpp (lines 1200 - 1201)


It seems like this framework is being spun up just to reserve resources on 
this agent. Do we not have a way to reserve the resources via the HTTP endpoint?


- Michael Park


On Jan. 20, 2016, 12:11 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> ---
> 
> (Updated Jan. 20, 2016, 12:11 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added reservation endpoint test without auth and principal.
> 
> Currently, dynamic reservation endpoints will not work when HTTP 
> authentication is not set. This test checks that these endpoints will fail as 
> expected in this case.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42368/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, 
> was added, and make check was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-19 Thread Greg Mann


> On Jan. 20, 2016, 12:44 a.m., Michael Park wrote:
> > src/tests/reservation_endpoints_tests.cpp, lines 1173-1184
> > 
> >
> > Why are we testing for this?
> 
> Greg Mann wrote:
> When we met previously, Vinod mentioned that we sometimes equate 
> default-initialized values with null values in the codebase, so I though it 
> worth testing that case here.

I dropped this case.


- Greg


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


On Jan. 20, 2016, 3:43 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> ---
> 
> (Updated Jan. 20, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added reservation endpoint test without auth and principal.
> 
> Currently, dynamic reservation endpoints will not work when HTTP 
> authentication is not set. This test checks that these endpoints will fail as 
> expected in this case.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42368/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, 
> was added, and make check was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-19 Thread Greg Mann

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

(Updated Jan. 20, 2016, 3:43 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Changes
---

Addressed comments.


Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195


Repository: mesos


Description
---

Added reservation endpoint test without auth and principal.

Currently, dynamic reservation endpoints will not work when HTTP authentication 
is not set. This test checks that these endpoints will fail as expected in this 
case.


Diffs (updated)
-

  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 

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


Testing
---

A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was 
added, and make check was used to test. The new test was also run with 
`--gtest_repeat=1000 --gtest_break_on_failure=1`.


Thanks,

Greg Mann



Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-16 Thread Greg Mann

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

(Updated Jan. 16, 2016, 4:31 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Changes
---

Added target version to comment.


Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195


Repository: mesos


Description
---

Added reservation endpoint test without auth and principal.

Currently, dynamic reservation endpoints will not work when HTTP authentication 
is not set. This test checks that these endpoints will fail as expected in this 
case.


Diffs (updated)
-

  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 

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


Testing
---

A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was 
added, and make check was used to test. The new test was also run with 
`--gtest_repeat=1000 --gtest_break_on_failure=1`.


Thanks,

Greg Mann



Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-15 Thread Greg Mann

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

Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195


Repository: mesos


Description
---

Added reservation endpoint test without auth and principal.

Currently, dynamic reservation endpoints will not work when HTTP authentication 
is not set. This test checks that these endpoints will fail as expected in this 
case.


Diffs
-

  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 

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


Testing
---

A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was 
added, and make check was used to test. The new test was also run with 
`--gtest_repeat=1000 --gtest_break_on_failure=1`.


Thanks,

Greg Mann



Re: Review Request 42368: Added reservation endpoint test without auth and principal.

2016-01-15 Thread Greg Mann

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

(Updated Jan. 15, 2016, 11:24 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195


Repository: mesos


Description
---

Added reservation endpoint test without auth and principal.

Currently, dynamic reservation endpoints will not work when HTTP authentication 
is not set. This test checks that these endpoints will fail as expected in this 
case.


Diffs
-

  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 

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


Testing
---

A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was 
added, and make check was used to test. The new test was also run with 
`--gtest_repeat=1000 --gtest_break_on_failure=1`.


Thanks,

Greg Mann