----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54221/#review157518 -----------------------------------------------------------
can you add a simple test for this change as discussed. comprehensive tests can come later. src/slave/http.cpp (line 518) <https://reviews.apache.org/r/54221/#comment228142> why is `principal` not being passed? also the signatures of the API handlers are a bit inconsistent now ``` # most of the handlers are foo(call, principal, contentType) attachContainerInput(contentType, acceptType, call, reader) attachContainerOutput(call, contentType, acceptType, principal) launchContainerOutput(call, contentType, acceptType, principal) ``` i propose we do ``` # NOTE: s/contentType/acceptType/ for consistency foo(call, acceptType, principal) attachContainerInput(call, reader, contentType, acceptType, principal) attachContainerOutput(call, contentType, acceptType, principal) launchContainerOutput(call, contentType, acceptType, principal) ``` i can do the change for `foo`. can you change `attachContainerInput` in this patch? src/slave/http.cpp (line 2172) <https://reviews.apache.org/r/54221/#comment228151> s/reader/decoder/ we have been naming recordio::Reader as `decoder` when it conflicts with the naming of Pipe::Reder. src/slave/http.cpp (line 2180) <https://reviews.apache.org/r/54221/#comment228152> if you change the above, this will be s/reader_/reader/ src/slave/http.cpp (line 2190) <https://reviews.apache.org/r/54221/#comment228160> Can you expand on why you need to write the first record manually, for posterity? src/slave/http.cpp (lines 2193 - 2194) <https://reviews.apache.org/r/54221/#comment228161> why do this here instead of inside the lambda? src/slave/http.cpp (line 2230) <https://reviews.apache.org/r/54221/#comment228163> This is different from how we prolong the lifetime of `connection` in `attachContainerOutput`. can we make them consistent? - Vinod Kone On Nov. 30, 2016, 10:09 p.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54221/ > ----------------------------------------------------------- > > (Updated Nov. 30, 2016, 10:09 p.m.) > > > Review request for mesos, Kevin Klues and Vinod Kone. > > > Bugs: MESOS-6472 > https://issues.apache.org/jira/browse/MESOS-6472 > > > Repository: mesos > > > Description > ------- > > Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent. > > > Diffs > ----- > > src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 > src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa > > Diff: https://reviews.apache.org/r/54221/diff/ > > > Testing > ------- > > make check (tests would be added soon and can't be committed without them) > > > Thanks, > > Anand Mazumdar > >
