----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42368/#review115715 -----------------------------------------------------------
src/tests/reservation_endpoints_tests.cpp (lines 1159 - 1160) <https://reviews.apache.org/r/42368/#comment176752> Can we leave the `TODO` here saying this should work and should be fixed in `0.28`? There are like 5 or 6 cases in this test, so the note at the top doesn't tell us which ones need to be updated. src/tests/reservation_endpoints_tests.cpp (line 1161) <https://reviews.apache.org/r/42368/#comment176749> Can we name these as `dynamicallyReservedWithNoPrincipal`, and for the rest as well? e.g. `dynamicallyReservedWithDefaultPrincipal`. src/tests/reservation_endpoints_tests.cpp (lines 1174 - 1176) <https://reviews.apache.org/r/42368/#comment176760> (1) `HTTP authorization`? -- Isn't it authentication? (2) `and dynamic reservations are currently unallowed without a principal`? -- What does this mean? We passed a `ReservationInfo` with a `principal` set here, and generally, passing `None()` as the authentication header isn't disallowed. src/tests/reservation_endpoints_tests.cpp (line 1192) <https://reviews.apache.org/r/42368/#comment176750> Can we call this: `dynamicallyReservedWithNonMatchingPrincipal`? "New principal" makes it sound as if we're in the context of trying to update an existing principal to a new principal or something. - Michael Park On Jan. 20, 2016, 6:59 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42368/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2016, 6:59 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 authentication. > > Currently, dynamic reservation endpoints will not work when HTTP > authentication is not set. Currently, the authentication code will always > return `None()` as the principal when authentication is disabled. > Furthermore, the `ReservationInfo.principal` field is being migrated to > `optional`, so dynamic reservations without a principal are invalidated by > the master. This test checks that these endpoints will fail as expected when > HTTP authentication is disabled. > > > 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 > >
