Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-26 Thread Alexander Rukletsov 


> On Aug. 26, 2016, 12:33 p.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.cpp, line 378
> > 
> >
> > "Killing the HTTP health check process "

Also fits one line afterwards.


- Alexander


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


On Aug. 25, 2016, 1:24 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 25, 2016, 1:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-26 Thread Alexander Rukletsov 

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


Fix it, then Ship it!





src/health-check/health_checker.cpp (lines 69 - 70)


Blank line



src/health-check/health_checker.cpp (lines 70 - 71)


// Use '127.0.0.1' instead of 'localhost', because the host
// file in some container images may not contain 'localhost'.



src/health-check/health_checker.cpp (line 71)


typo: container



src/health-check/health_checker.cpp (lines 333 - 337)


How about this for readability:
```
  string scheme = http.has_scheme() ? http.scheme() : DEFAULT_HTTP_SCHEME;
  string path = http.has_path() ? http.path() : "";
  string url = scheme + "://" + DEFAULT_DOMAIN + ":" + 
stringify(http.port()) + path;
```



src/health-check/health_checker.cpp (line 345)


I'm not sure whether this is good or not, let's leave it for now. If there 
will be user feedback, we can fix it later.



src/health-check/health_checker.cpp (line 360)


s/exec/create



src/health-check/health_checker.cpp (line 378)


"Killing the HTTP health check process "



src/health-check/health_checker.cpp (line 384)


"curl has not returned after " + stringify(timeout) + "; aborting"



src/health-check/health_checker.cpp (lines 390 - 392)


For readability, let's reformat like in the declaration:
```
const std::tuple<
process::Future

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-25 Thread haosdent huang 

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

(Updated Aug. 25, 2016, 1:24 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-25 Thread haosdent huang 

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

(Updated Aug. 25, 2016, 12:34 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-24 Thread haosdent huang 

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

(Updated Aug. 24, 2016, 5:23 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-24 Thread haosdent huang 

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

(Updated Aug. 24, 2016, 4:52 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-24 Thread haosdent huang 

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

(Updated Aug. 24, 2016, 4:50 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-17 Thread haosdent huang 


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 330
> > 
> >
> > Why do you need to discard original future here?
> 
> haosdent huang wrote:
> I saw we discard original future in every `after` callback. So I discard 
> it here as well.
> 
> Alexander Rukletsov wrote:
> This is may be true. Do you know why we do this?
> 
> haosdent huang wrote:
> I think it is because the `.then` callback may continue to executed when 
> the `future` is ready if we don't call `future.discard()`.
> 
> Alexander Rukletsov wrote:
> That's correct.

Should I add a comment here?


- haosdent


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


On Aug. 15, 2016, 3:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 15, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-17 Thread haosdent huang 


> On Aug. 15, 2016, 12:43 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.cpp, lines 363-367
> > 
> >
> > Why do you think this won't affect ibprocess communication? Actors' 
> > callbacks are executed on worker threads, so it's not clear for which 
> > thread `setns` will be called. It would have worked if we could pin actors 
> > to dedicated threads.

Agree we should not setns in a libprocess context. Let me updated to use 
another way.


- haosdent


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


On Aug. 15, 2016, 3:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 15, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-16 Thread Alexander Rukletsov 


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 330
> > 
> >
> > Why do you need to discard original future here?
> 
> haosdent huang wrote:
> I saw we discard original future in every `after` callback. So I discard 
> it here as well.
> 
> Alexander Rukletsov wrote:
> This is may be true. Do you know why we do this?
> 
> haosdent huang wrote:
> I think it is because the `.then` callback may continue to executed when 
> the `future` is ready if we don't call `future.discard()`.

That's correct.


- Alexander


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


On Aug. 15, 2016, 3:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 15, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-16 Thread Alexander Rukletsov 

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




src/health-check/health_checker.cpp (lines 362 - 363)


Instead of `setns` here, how about creating a subprocess hook and do it 
there? This way we guarantee there is a single thread (libprocess has not been 
initialized yet) and that the network namespace is set before `curl` invocation.


- Alexander Rukletsov


On Aug. 15, 2016, 3:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 15, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-15 Thread haosdent huang 


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 330
> > 
> >
> > Why do you need to discard original future here?
> 
> haosdent huang wrote:
> I saw we discard original future in every `after` callback. So I discard 
> it here as well.
> 
> Alexander Rukletsov wrote:
> This is may be true. Do you know why we do this?

I think it is because the `.then` callback may continue to executed when the 
`future` is ready if we don't call `future.discard()`.


- haosdent


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


On Aug. 15, 2016, 3:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 15, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-15 Thread haosdent huang 

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

(Updated Aug. 15, 2016, 3:42 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-14 Thread Alexander Rukletsov 

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




src/health-check/health_checker.cpp (lines 363 - 367)


Why do you think this won't affect ibprocess communication? Actors' 
callbacks are executed on worker threads, so it's not clear for which thread 
`setns` will be called. It would have worked if we could pin actors to 
dedicated threads.


- Alexander Rukletsov


On Aug. 14, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 14, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-14 Thread Alexander Rukletsov 


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 330
> > 
> >
> > Why do you need to discard original future here?
> 
> haosdent huang wrote:
> I saw we discard original future in every `after` callback. So I discard 
> it here as well.

This is may be true. Do you know why we do this?


- Alexander


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


On Aug. 14, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 14, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-14 Thread haosdent huang 

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

(Updated Aug. 14, 2016, 5:05 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-13 Thread haosdent huang 

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

(Updated Aug. 13, 2016, 8:46 a.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-13 Thread haosdent huang 

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

(Updated Aug. 13, 2016, 6:47 a.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Changes
---

Address @alexr's comments.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

Diff: https://reviews.apache.org/r/36816/diff/


Testing (updated)
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-13 Thread haosdent huang 


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 280
> > 
> >
> > Do you think we should add "/" if the user has not specified anything?

Use "" instead, thx!


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 330
> > 
> >
> > Why do you need to discard original future here?

I saw we discard original future in every `after` callback. So I discard it 
here as well.


> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 332
> > 
> >
> > It's unclear, what exactly is pending. How about: "CURL has not 
> > returned after " + stringify(timeout) + "; aborting"

I change to `"Aborting because curl has not returned after " + 
stringify(timeout))`


- haosdent


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


On Aug. 7, 2016, 6:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 7, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
>   src/health-check/health_checker.cpp 
> 585a0b565d948cfa292bad818a710501a4ce0daf 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-10 Thread Alexander Rukletsov 

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




src/health-check/health_checker.hpp (line 275)


It's good to have a default. Let's create a class constant 
`DEFAULT_HTTP_SCHEME`.



src/health-check/health_checker.hpp (lines 276 - 278)


This belongs into the `validate()` function.



src/health-check/health_checker.hpp (line 280)


Do you think we should add "/" if the user has not specified anything?



src/health-check/health_checker.hpp (line 282)


Again, it's great we can deduce the domain for the user, but let's put into 
the class constant, e.g. `DEFAULT_DOMAIN`.



src/health-check/health_checker.hpp (line 304)


We can remove "with reason", what do you think?



src/health-check/health_checker.hpp (lines 320 - 333)


This is used once, maybe it makes sense to replace it with a lambda then? 
The function is short enough, it's declaration is much longer than the function 
itself.



src/health-check/health_checker.hpp (line 330)


Why do you need to discard original future here?



src/health-check/health_checker.hpp (line 332)


It's unclear, what exactly is pending. How about: "CURL has not returned 
after " + stringify(timeout) + "; aborting"



src/health-check/health_checker.hpp (line 356)


We should be consistent: either wrap `curl` in '' everywhere or nowhere.



src/health-check/health_checker.hpp (lines 376 - 377)


Let's swap these conditions : )



src/health-check/health_checker.hpp (lines 392 - 393)


I would omit "with reason", ':' is enough.

Let's make sure we test these paths.



src/health-check/health_checker.cpp (lines 57 - 59)


Let's introduce a separate `validate` function. See my comments in the 
previous review.


- Alexander Rukletsov


On Aug. 7, 2016, 6:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 7, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
>   src/health-check/health_checker.cpp 
> 585a0b565d948cfa292bad818a710501a4ce0daf 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-07 Thread haosdent huang 

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

(Updated Aug. 7, 2016, 6:21 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Changes
---

Use curl instead of libprocess.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
  src/health-check/health_checker.cpp 585a0b565d948cfa292bad818a710501a4ce0daf 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-04 Thread haosdent huang 

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




src/health-check/health_checker.hpp (line 283)


We should use `libcurl` to execute http health check to avoid our https 
health check depends on SSL compile flag.


- haosdent huang


On Aug. 1, 2016, 1:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 1, 2016, 1:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 590e169108b2ce5881734ec7f4b01cef9937461a 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-03 Thread Gastón Kleiman 

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




include/mesos/v1/mesos.proto (line 339)


Marathon does the following for health checks:

> A health check is considered passing ifits HTTP response code is between 
200 and 399, inclusive.

What do you think about adopting this as the default behaviour and later 
adding support for specifying healthy/unhealthy response code ranges?



src/health-check/health_checker.hpp (lines 283 - 284)


We should also document if libprocess validates the certificate or not.

If it does try to validate the certificate, then using `localhost` for the 
domain might be a problem.


- Gastón Kleiman


On Aug. 1, 2016, 1:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 1, 2016, 1:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 590e169108b2ce5881734ec7f4b01cef9937461a 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-02 Thread Alexander Rukletsov 

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




src/health-check/health_checker.hpp (lines 283 - 284)


We should make sure we document that HTTPS health checks are only supported 
if SSL is enabled! This is not obvious!


- Alexander Rukletsov


On Aug. 1, 2016, 1:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 1, 2016, 1:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 590e169108b2ce5881734ec7f4b01cef9937461a 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-02 Thread Alexander Rukletsov 

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




include/mesos/mesos.proto (lines 324 - 346)


How about this message?
```
// Describes an HTTP health check.
 message HTTPx {
   enum Protocol {
 UNKNOWN = 0;
 HTTP = 1;
 HTTPS = 2;
   }

   required Protocol protocol = 1 [default = HTTP];

   // Port to send the HTTPx request.
   required uint32 port = 2;

   // HTTPx request path.
   optional string path = 3 [default = "/"];

   // This field is post-MVP. While adding POST and PUT is simple,
   // supporting payload is more involved.
   optional HTTPMethod method = 4 [default = GET];

   // This field is post-MVP. Additional HTTP headers that should
   // be set when sending the health check request.
   repeated HTTPHeader headers = 5;

   // Expected response statuses. Not specifying any statuses implies
   // that any returned status is acceptable.
   //
   // NOTE: Nomad uses two sets of statuses: one considered warnings
   // and other failures.
   //
   // NOTE: In the MVP, we can treat return codes between 200 & 399
   // as success, and failure otherwise.
   repeated uint32 statuses = 6;

   // TODO(benh): Include an 'optional bytes data' field for checking
   // for specific data in the response.
 }
```



include/mesos/mesos.proto (line 326)


I suggest we name it `HTTPx` to reflect we also support HTTPS.



include/mesos/mesos.proto (line 335)


Let's use an enum instead of the flag (see general proposal).



include/mesos/mesos.proto (line 342)


What would this be if not "localhost"? What other values are possible here? 
How do we deal with virtual networks?

Can't we always deduce it and get rid of this field altogether?



include/mesos/mesos.proto (lines 366 - 368)


I suggest we use a "union trick":
```
message HealthCheckInfo {
 enum Type {
   UNKNOWN = 0;
   COMMAND = 1;
   HTTPX = 2;
   TCP = 3;
 }
...
 required Type type = 8;
 optional CommandInfo command = 7;
 optional HTTPx http = 1;
 optional Socket socket = 9;
}



include/mesos/v1/mesos.proto (line 339)


I'm not sure we should tackle this now. It seems that we may want to 
support ranges here. Additionally, we may want to use two separate ranges for 
"unhealthy" and "not that healthy tasks". I would say this needs more thinking 
and hence propose to punt on this in this patch (we will return to it right 
after).

For now my usggestion would be to use 200 and 399 as success and everything 
else as failure.



src/health-check/health_checker.hpp (lines 273 - 276)


Why do you want to check it every time a health check is performed? It 
seems like validation for me, which should be done once in the beginning.



src/health-check/health_checker.hpp (line 279)


Will we always have a domain name, I can imagine situations when all we 
have is an ip, which we have to deduce from `ContainerInfo` and `PortMappings`.

Having said that, I think we can go with `localhost`, though it will be 
slightly complicated for the docker container, but I don't think we should 
expose it in protobufs.



src/health-check/health_checker.hpp (line 281)


... HTTP health check ...



src/health-check/health_checker.hpp (line 283)


s/query/httpResponse ?



src/health-check/health_checker.hpp (line 301)


s/code/httpStatusCode



src/health-check/health_checker.hpp (lines 302 - 303)


This can be rather costly. If we want to support allow status codes, we 
should build an appropriate data structure (map or hash table) in the beginning 
and look up there. However, see above my comments about this proto field.



src/health-check/health_checker.hpp (lines 307 - 308)


Blank line



src/health-check/health_checker.hpp (lines 313 - 315)


If you accept my "union trick" suggestion, you can check the type here and 
make sure the appropriate field is set. See what we do for

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-01 Thread haosdent huang 

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

(Updated Aug. 1, 2016, 1:15 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 590e169108b2ce5881734ec7f4b01cef9937461a 
  include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
  src/health-check/health_checker.hpp b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-08-01 Thread haosdent huang 

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

(Updated Aug. 1, 2016, 1:05 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 590e169108b2ce5881734ec7f4b01cef9937461a 
  include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
  src/health-check/health_checker.hpp b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-07-16 Thread Tomasz Janiszewski 

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


Ship it!




Ship It!

- Tomasz Janiszewski


On July 2, 2016, 3:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated July 2, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/health-check/health_checker.hpp 
> f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-07-02 Thread haosdent huang 

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

(Updated July 2, 2016, 3:07 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-06-30 Thread haosdent huang 

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

(Updated June 30, 2016, 11:50 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-06-29 Thread haosdent huang 

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

(Updated June 29, 2016, 9:04 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
Timothy Chen.


Changes
---

Rebase.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-06-29 Thread haosdent huang 


> On June 29, 2016, 5:02 a.m., Timothy Chen wrote:
> > Can you add a test?

@tnachen We have two test cases before. But I think `health_check_tests.cpp` a 
bit mess up now, I am try to reorganize it to make it more clear.


- haosdent


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


On June 29, 2016, 4:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated June 29, 2016, 4:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-06-28 Thread Timothy Chen 

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



Can you add a test?

- Timothy Chen


On June 29, 2016, 4:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated June 29, 2016, 4:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

2016-06-28 Thread haosdent huang 

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

(Updated June 29, 2016, 4:45 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
Timothy Chen.


Changes
---

Rebase.


Summary (updated)
-

Supported HTTP/HTTPS in health check.


Bugs: MESOS-2533
https://issues.apache.org/jira/browse/MESOS-2533


Repository: mesos


Description (updated)
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 

Diff: https://reviews.apache.org/r/36816/diff/


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang