> On Dec. 1, 2016, 7:49 p.m., Anand Mazumdar wrote: > > src/slave/http.cpp, line 1982 > > <https://reviews.apache.org/r/54196/diff/2/?file=1573665#file1573665line1982> > > > > Why not be explicit here and set it to `ContainerClass::DEFAULT` like > > we do for the corresponding session one?
done. > On Dec. 1, 2016, 7:49 p.m., Anand Mazumdar wrote: > > src/slave/http.cpp, line 2197 > > <https://reviews.apache.org/r/54196/diff/2/?file=1573665#file1573665line2197> > > > > Nit: s/goes way/is interrupted i did s/goes way/breaks/ > On Dec. 1, 2016, 7:49 p.m., Anand Mazumdar wrote: > > src/slave/http.cpp, lines 2234-2243 > > <https://reviews.apache.org/r/54196/diff/2/?file=1573665#file1573665line2234> > > > > hmm, can we create a lambda destroy and just invoke it with a `message` > > argument rather then duplicating it here and else where? passing the message looks a bit ugly because we need to construct it with "+" and stringify(). so i opted to log the warning and call the lambda instead. let me know if that looks ok. > On Dec. 1, 2016, 7:49 p.m., Anand Mazumdar wrote: > > src/slave/http.cpp, line 2229 > > <https://reviews.apache.org/r/54196/diff/2/?file=1573665#file1573665line2229> > > > > Can we resolve this TODO? This can happen often if the launched > > container as part of session terminates. This would result in an EOF. > > > > You might want to sync up with Kevin regarding other cases when this > > can happen. as discussed offline, if the container terminates we would get a failure instead of EOF here? but anyway, leveraged trick mentioned by mpark to combine the handlers. > On Dec. 1, 2016, 7:49 p.m., Anand Mazumdar wrote: > > src/tests/api_tests.cpp, line 3623 > > <https://reviews.apache.org/r/54196/diff/2/?file=1573668#file1573668line3623> > > > > hmm, this test would break the moment Jie commits Kevin's changes to > > add support to the containerizer for launching debug containers? I reealized I used "TestContainerizer" and not "MesosContainerizer" here. So nothing broke after Jie's changes. I would like to use TestContainerizer because it lets me test more functionality. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54196/#review157616 ----------------------------------------------------------- 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 > >
