> On Sept. 22, 2016, 5:23 p.m., Vinod Kone wrote: > > src/tests/api_tests.cpp, lines 3235-3280 > > <https://reviews.apache.org/r/52135/diff/2/?file=1507935#file1507935line3235> > > > > Hmm. I would put them in their own separate tests since the setup for > > them is easy enough.
Sounds good. > On Sept. 22, 2016, 5:23 p.m., Vinod Kone wrote: > > src/slave/http.cpp, lines 1977-1979 > > <https://reviews.apache.org/r/52135/diff/2/?file=1507934#file1507934line1977> > > > > I'm wondering if we should include the whole ContainerTermination proto > > in the response instead of just the status. For example, the "message" and > > "reasons" might be useful if the container OOMs. > > > > Thoughts? Agreed, I'll punt on this for a separate follow-up. My thinking for omitting them was that TaskState.Reason seemed unfortunately wide (many values are not applicable here). So, I was hoping we could introduce a container specific termination reason (OOM, OOD, etc). Note that right now the resources are not used, so it won't be useful just yet. For message, we could add it but I'd like to add it ideally after we have the Reason in place (OOM, OOD, etc), to avoid users parsing / relying on the message format (that might change underneath them). - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52135/#review150029 ----------------------------------------------------------- On Sept. 22, 2016, 6:06 a.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52135/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2016, 6:06 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-2449 > https://issues.apache.org/jira/browse/MESOS-2449 > > > Repository: mesos > > > Description > ------- > > This patch adds the wiring for the *_NESTED_CONTAINER calls, > including validation and calling into the containerizer. > > > Diffs > ----- > > src/slave/http.cpp 0aae14583faf95fee678b3a3424c96cdba531968 > src/tests/api_tests.cpp 26f99f7c337fbbc5278d1b30d3d5c8f659ddf5ca > > Diff: https://reviews.apache.org/r/52135/diff/ > > > Testing > ------- > > Added a test that mocks out the containerizer. > > > Thanks, > > Benjamin Mahler > >
