Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/#review158317 --- Ship it! Ship It! - Adam B On Dec. 7, 2016, 1:50 a.m.,

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-07 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/ --- (Updated Dec. 7, 2016, 10:50 a.m.) Review request for mesos, Adam B, Kevin

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-07 Thread Alexander Rojas
> On Dec. 7, 2016, 1:26 a.m., Adam B wrote: > > src/slave/http.cpp, line 2398 > > > > > > Nit: You use this value 3 times. Maybe worth pulling it out into a > > variable. There is a reason, `has_container_id()`

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/#review158302 --- Patch looks great! Reviews applied: [54381] Passed command:

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Adam B
> On Dec. 5, 2016, 4:55 p.m., Vinod Kone wrote: > > src/tests/api_tests.cpp, lines 3728-3793 > > > > > > why did you change these tests? the idea with the tests was to verify > > that `Containerizer::attach()` was

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Adam B
> On Dec. 6, 2016, 12:50 a.m., Adam B wrote: > > src/slave/http.cpp, lines 2465-2468 > > > > > > Can you please comment on the need for std::move here? > > Why is decoder a `&&` in the first place? @vinodkone

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/#review158255 --- A few nits, but we really just need Vinod's approval of the test

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/ --- (Updated Dec. 6, 2016, 3:15 p.m.) Review request for mesos, Adam B, Kevin

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Adam B
> On Dec. 5, 2016, 4:55 p.m., Vinod Kone wrote: > > src/tests/api_tests.cpp, lines 3728-3793 > > > > > > why did you change these tests? the idea with the tests was to verify > > that `Containerizer::attach()` was

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-06 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/#review158132 --- Looks pretty good to me. Continuing test discussion in that

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-05 Thread Alexander Rojas
> On Dec. 6, 2016, 1:55 a.m., Vinod Kone wrote: > > src/tests/api_tests.cpp, lines 3728-3793 > > > > > > why did you change these tests? the idea with the tests was to verify > > that `Containerizer::attach()` was

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/#review158090 --- src/tests/api_tests.cpp (lines 3728 - 3785)

Re: Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-05 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/#review158056 --- Patch looks great! Reviews applied: [54381] Passed command:

Review Request 54381: Adds authorization support when attaching containers input/output.

2016-12-05 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54381/ --- Review request for mesos, Adam B, Kevin Klues, and Vinod Kone. Repository: