> On March 20, 2017, 8:52 p.m., Joseph Wu wrote: > > src/checks/checker.cpp > > Lines 440-443 (original), 440-448 (patched) > > <https://reviews.apache.org/r/57775/diff/2/?file=1668401#file1668401line440> > > > > I'm guessing there's something in this chain that necessitates this > > change. Can you explain? > > > > At a glance, these macros should work on Windows: > > ``` > > #ifndef WIFEXITED > > #define WIFEXITED(x) ((x) != -1) > > #endif // WIFEXITED > > > > #ifndef WEXITSTATUS > > #define WEXITSTATUS(x) (x & 0xFF) > > #endif // WEXITSTATUS > > ```
We would like to return the exit code of the check, see https://github.com/apache/mesos/blob/05e9a1d40572b8383a582e15663d861b134a7dad/include/mesos/mesos.proto#L1738-L1743 . We obviously want something portable, so that schedulers don't have to interpret this value differently based on the OS the agent is running on. However, AFAIK the way exit code is obtained differs on Unix and Windows, see https://issues.apache.org/jira/browse/MESOS-7242. Your macros look fine at a first glance, but I don't want to introduce them locally. Can we tackle this problem in general and fix MESOS-7242 instead? > On March 20, 2017, 8:52 p.m., Joseph Wu wrote: > > src/tests/check_tests.cpp > > Lines 747-750 (patched) > > <https://reviews.apache.org/r/57775/diff/2/?file=1668402#file1668402line747> > > > > I don't recognize this test (so I assume it's been added in this > > chain)... > > > > Can you assign the TODO to yourself? Or to me (`josephw`). It is indeed a new test. Here is where this TODO is coming from. I can assign both to you : ). - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57775/#review169499 ----------------------------------------------------------- On March 20, 2017, 5:02 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57775/ > ----------------------------------------------------------- > > (Updated March 20, 2017, 5:02 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Gastón Kleiman, and Vinod > Kone. > > > Bugs: MESOS-6906 > https://issues.apache.org/jira/browse/MESOS-6906 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/checks/checker.cpp 314354cc374b453ec12e25e3d4730a92697468cf > src/tests/check_tests.cpp f035c16920deaf559420ae0d7d881330ff65ae44 > > > Diff: https://reviews.apache.org/r/57775/diff/2/ > > > Testing > ------- > > make check on Linux and Windows > > > Thanks, > > Alexander Rukletsov > >
