----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54196/#review157697 -----------------------------------------------------------
Fix it, then Ship it! LGTM minus a small query regarding logging a warning message later. src/slave/http.cpp (line 2426) <https://reviews.apache.org/r/54196/#comment228316> 2 line indent src/slave/http.cpp (line 2455) <https://reviews.apache.org/r/54196/#comment228321> Nit: s/and log on failure// src/slave/http.cpp (line 2456) <https://reviews.apache.org/r/54196/#comment228317> Can you explicity capture here? src/slave/http.cpp (lines 2514 - 2516) <https://reviews.apache.org/r/54196/#comment228318> hmm, this message seems a bit mis-leading. What happens if the command that was run in the debug container entrypoint completes? We would still log this warning message? src/tests/api_tests.cpp (line 3641) <https://reviews.apache.org/r/54196/#comment228319> Kill this. src/tests/api_tests.cpp (line 3653) <https://reviews.apache.org/r/54196/#comment228320> Kill this. src/tests/api_tests.cpp (line 3683) <https://reviews.apache.org/r/54196/#comment228323> s/callbacks/pending callbacks - Anand Mazumdar On Dec. 2, 2016, 4:42 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54196/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2016, 4:42 a.m.) > > > Review request for mesos and Anand Mazumdar. > > > Bugs: MESOS-6471 > https://issues.apache.org/jira/browse/MESOS-6471 > > > Repository: mesos > > > Description > ------- > > In addition to launching the nested container the API handler > ensures that the container is destroyed if the connection breaks. > > > Diffs > ----- > > src/slave/http.cpp ace3575fd3a330f788e678283bc05e806cf9b264 > src/slave/slave.hpp cfec9dd5fa3550ba1192fc86b1ff0d73ee1671e0 > src/slave/validation.cpp 4005cfcd8bcdb923a73de471100c3567344234cf > src/tests/api_tests.cpp ea6e037c18fd116eff473ee246faa504ec37b7da > > Diff: https://reviews.apache.org/r/54196/diff/ > > > Testing > ------- > > make check > > Added a basic test for now that tests the failure case. Will be adding more > tests in subsequent reviews. > > > Thanks, > > Vinod Kone > >
