> On Feb. 9, 2017, 3:20 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp, lines 653-654 > > <https://reviews.apache.org/r/55901/diff/8/?file=1627619#file1627619line653> > > > > Why not `defer`?
Added `defer` where it corresponds. > On Feb. 9, 2017, 3:20 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp, line 635 > > <https://reviews.apache.org/r/55901/diff/8/?file=1627619#file1627619line635> > > > > Could you please specify what this function returns? Status or exit > > code? This depends on what containerizer the agent is using. In the case of the Mesos containerizer, it currently returns a status code. > On Feb. 9, 2017, 3:20 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp, line 667 > > <https://reviews.apache.org/r/55901/diff/8/?file=1627619#file1627619line667> > > > > What do you get from the agent call: status or exit code directly? It's > > probably not that important here, but it is for checks. This depends on what containerizer the agent is using. In the case of the Mesos containerizer, it currently returns a status code. > On Feb. 9, 2017, 3:20 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp, lines 565-567 > > <https://reviews.apache.org/r/55901/diff/8/?file=1627619#file1627619line565> > > > > This should probably be indented, no? Fixed. > On Feb. 9, 2017, 3:20 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp, line 555 > > <https://reviews.apache.org/r/55901/diff/8/?file=1627619#file1627619line555> > > > > s/until/until after/ ? Fixed. > On Feb. 9, 2017, 3:20 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp, line 615 > > <https://reviews.apache.org/r/55901/diff/8/?file=1627619#file1627619line615> > > > > s/this future is completed//we finish processing current timeout./ > > > > It's not clear what "this future" means. Fixed. - Gastón ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55901/#review164919 ----------------------------------------------------------- On Feb. 10, 2017, 6:40 p.m., Gastón Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55901/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2017, 6:40 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, and Vinod Kone. > > > Bugs: MESOS-6280 > https://issues.apache.org/jira/browse/MESOS-6280 > > > Repository: mesos > > > Description > ------- > > Added support for command health checks to the default executor. > > > Diffs > ----- > > src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 > src/checks/health_checker.cpp a5225ff1f4b071ed4182d41fa8ecc705fa4dbe00 > src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a > src/tests/health_check_tests.cpp 7b6a803a28b2e4f6c27e9a0c4f668350ec2d5a81 > > Diff: https://reviews.apache.org/r/55901/diff/ > > > Testing > ------- > > Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It > passes on Linux, but not on macOS, because of MESOS-7050. > > > Thanks, > > Gastón Kleiman > >
