Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-06 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/ --- (Updated March 6, 2017, 7:11 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-03 Thread Vinod Kone
> On March 4, 2017, 12:31 a.m., Vinod Kone wrote: > > src/master/http.cpp > > Lines 482 (patched) > > > > > > Maybe `BadRequest` is better here than `Forbidden`. For example we send > > `BadRequest` below in

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-03 Thread Greg Mann
> On March 4, 2017, 12:31 a.m., Vinod Kone wrote: > > src/master/http.cpp > > Lines 482 (patched) > > > > > > Maybe `BadRequest` is better here than `Forbidden`. For example we send > > `BadRequest` below in

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/#review167908 --- src/master/http.cpp Lines 482 (patched)

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-03 Thread Vinod Kone
> On March 2, 2017, 10:41 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 1565-1573 (original), 1569-1577 (patched) > > > > > > CHECK(principal->value.isSome()) > > Greg Mann wrote: > In this case,

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-03 Thread Greg Mann
> On March 2, 2017, 10:41 p.m., Greg Mann wrote: > > src/master/master.hpp > > Line 746 (original), 746 (patched) > > > > > > See if we can typedef this. Sounds like we can't :( > On March 2, 2017, 10:41 p.m.,

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-03 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/ --- (Updated March 3, 2017, 6:45 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-02 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/#review167754 --- src/master/http.cpp Lines 841-845 (original), 844-851 (patched)

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-03-02 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/#review167752 --- src/master/http.cpp Lines 741-744 (original), 742-745 (patched)

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-02-28 Thread Greg Mann
> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote: > > src/master/http.cpp, lines 2224-2227 > > > > > > This line didn't need modification. However it does raises the issue of > > the signature of

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-02-28 Thread Greg Mann
> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote: > > src/master/http.cpp, line 115 > > > > > > wasn't metrics process used at all? It wasn't - I split this change out into a separate review:

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-02-28 Thread Greg Mann
> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote: > > src/master/http.cpp, lines 889-891 > > > > > > I'm not sure printing the whole `principal` with name and claims is > > such a good idea. The reason is that

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

2017-02-28 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/ --- (Updated Feb. 28, 2017, 6:34 p.m.) Review request for mesos, Adam B, Alexander