Re: Review Request 67207: Windows: Changed health check image to be an environment variable.

[Andrew Schwartzmeyer]

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


Shouldn't we have a "safe" default?



src/tests/environment.cpp
Line 372 (original), 373 (patched)


These commands are semantically different. Did you want `ping` to run for 
~60 seconds and stop?



src/tests/environment.cpp
Lines 396-403 (patched)


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)


If you wanted, you could `strings::join(" ", {docker->get(), "-H", ...});`



src/tests/environment.cpp
Lines 397-405 (original), 407-413 (patched)


With [#67188](https://reviews.apache.org/r/67188/), you couls `Try 
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
> 
>

Review Request 67207: Windows: Changed health check image to be an environment variable.

[Akash Gupta]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67207/
---

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