----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67207/#review203427 -----------------------------------------------------------
src/checks/checker_process.cpp Lines 1372-1374 (patched) <https://reviews.apache.org/r/67207/#comment285640> Shouldn't we have a "safe" default? src/tests/environment.cpp Line 372 (original), 373 (patched) <https://reviews.apache.org/r/67207/#comment285641> These commands are semantically different. Did you want `ping` to run for ~60 seconds and stop? src/tests/environment.cpp Lines 396-403 (patched) <https://reviews.apache.org/r/67207/#comment285643> Definitely still feel like erroring if a new environment variable is unset isn't something we should do... src/tests/environment.cpp Lines 397-399 (original), 407-409 (patched) <https://reviews.apache.org/r/67207/#comment285642> If you wanted, you could `strings::join(" ", {docker->get(), "-H", ...});` src/tests/environment.cpp Lines 397-405 (original), 407-413 (patched) <https://reviews.apache.org/r/67207/#comment285645> With [#67188](https://reviews.apache.org/r/67188/), you couls `Try<string> inspect os::shell(paths::join(...));` and then `if (inspect.isError()) { return Error(); }` Sadly though, due to the API, if the command errors, you lose all its output. So maybe not much better `os::system`. - Andrew Schwartzmeyer On May 18, 2018, 2:07 a.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67207/ > ----------------------------------------------------------- > > (Updated May 18, 2018, 2:07 a.m.) > > > Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and > Gaston Kleiman. > > > Bugs: MESOS-8499 > https://issues.apache.org/jira/browse/MESOS-8499 > > > Repository: mesos > > > Description > ------- > > To support the health check on different Windows hosts (1709, 1803...), > the health check image is now passed in as an environment variable > instead of being hardcoded. The commands have also been changed to use > the same one as Linux (`curl` and `mesos-tcp-connect`), since the image > is now controlled by the user. > > > Diffs > ----- > > src/checks/checker_process.hpp 228ac78715f120b43db05b5691412e8f5520de3b > src/checks/checker_process.cpp 7e484510b4fc2b94d4d4385dfa8f68bdd76e6093 > src/tests/environment.cpp a30592ac6b0002dad0947086ecbfdf4e2db62da5 > src/tests/health_check_tests.cpp c4661b4f005c18a7cdc69ce05576bcccb79d5dee > > > Diff: https://reviews.apache.org/r/67207/diff/1/ > > > Testing > ------- > > > Thanks, > > Akash Gupta > >
