----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/#review106892 -----------------------------------------------------------
src/health-check/main.cpp (line 238) <https://reviews.apache.org/r/36816/#comment165698> Seems weird to perform a http check with not statuses to check. Instead of returning silently I think we should send an error instead since the user might as well don't perform this check right? src/health-check/main.cpp (line 243) <https://reviews.apache.org/r/36816/#comment165696> Please log VLOG(2) the http url we're trying to health check src/health-check/main.cpp (line 263) <https://reviews.apache.org/r/36816/#comment165697> Response should also have a code uint32_t field now, you can check that directly without checking strings. src/health-check/main.cpp (line 295) <https://reviews.apache.org/r/36816/#comment165695> This formatting problem somehow slipped in, we need to align the << in both lines. src/tests/health_check_tests.cpp (line 930) <https://reviews.apache.org/r/36816/#comment165699> Can you also test that it fails the health check correctly when status doesn't match? - Timothy Chen On Oct. 3, 2015, 6:01 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36816/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2015, 6:01 p.m.) > > > Review request for mesos, Adam B, Michael Park, and Timothy Chen. > > > Bugs: MESOS-2533 > https://issues.apache.org/jira/browse/MESOS-2533 > > > Repository: mesos > > > Description > ------- > > Support HTTP checks in Mesos health check program > > > Diffs > ----- > > include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 > src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade > src/health-check/main.cpp 97b25716335ec5719c1100bd73d06b7fc98036c9 > src/tests/health_check_tests.cpp ff6275b19206b49eacb6761f3aeb58dd87651ade > > Diff: https://reviews.apache.org/r/36816/diff/ > > > Testing > ------- > > * Add a new unit test: HealthCheckTest.HealthyTaskThroughHttp > make check > > > Thanks, > > haosdent huang > >