mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rukletsov <ruklet...@gmail.com>
Subject Re: Review Request 57775: Ensured check's exit code is interpreted correctly on Windows.
Date Tue, 21 Mar 2017 15:43:07 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message