Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Oct. 23, 2015, 4:24 a.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/decoder.hpp 53b8079968a0145651bad9d11aa4be2b504de57a 
  3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
11e9217343c2c97da7bc392e426f5112f88fe43d 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 
  3rdparty/libprocess/src/tests/process_tests.cpp 
708b8d9a9f4987ddec5406233666ae3f2c93eb07 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

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

Ship it!


Thanks for your patience Tim! Looks good, just some minor comments (let's 
remove isStatus now that we don't have to do the casting stuff).


3rdparty/libprocess/include/process/http.hpp (lines 416 - 419)


This is no longer necessary, yes? Callers can just do an equality check 
against 'code'.



3rdparty/libprocess/src/decoder.hpp (lines 444 - 447)


Looks like this check should take place before setting the code and status, 
since it's validating the code.



3rdparty/libprocess/src/decoder.hpp (lines 656 - 659)


Ditto here, can you move this above the setting of the code and status?



3rdparty/libprocess/src/http.cpp (line 121)


Brace on newline :)



3rdparty/libprocess/src/tests/benchmarks.cpp (line 264)


Per my comment above, let's remove isStatus and just directly do equality 
against the 'code' field. Ditto for the other cases below.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 128 - 130)


Could you use the -> operator here and in all of the code below to avoid 
the .get(). mess? Thanks Tim!


- Ben Mahler


On Oct. 8, 2015, 11:05 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Oct. 8, 2015, 11:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/decoder.hpp 
> 53b8079968a0145651bad9d11aa4be2b504de57a 
>   3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Oct. 8, 2015, 11:05 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/decoder.hpp 53b8079968a0145651bad9d11aa4be2b504de57a 
  3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
  3rdparty/libprocess/src/tests/process_tests.cpp 
ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Oct. 6, 2015, 9:53 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, line 76
> > 
> >
> > We avoid static non-POD types due to destruction issues. Can you put 
> > this on the heap?
> > 
> > Per my suggestion above, perhaps we can have `Status::string(uint16_t 
> > code)` as a static function on the `Status` class that provides the string 
> > version of the status.
> > 
> > This should clean up the tests as well.

I'm not sure how this will clean up the tests since you need to still check if 
the code is in the statuses map, I think it just avoids the lookup yourself.
Anyhow I'm going to update the review as you suggested.


- Timothy


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


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/decoder.hpp 
> 53b8079968a0145651bad9d11aa4be2b504de57a 
>   3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Oct. 6, 2015, 8:27 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/decoder.hpp 53b8079968a0145651bad9d11aa4be2b504de57a 
  3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
  3rdparty/libprocess/src/tests/process_tests.cpp 
ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

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



3rdparty/libprocess/include/process/http.hpp (lines 146 - 188)


What is the value of this being an enum? We can't store the enum anywhere 
so every time we want to use these we have to do explicit casting, like you 
have in Response below.

How about making this a class called 'Status' which holds each of these as 
a static member variable?

That way we can still do:

```
Status::OK
```

And no explicit casting is necessary.



3rdparty/libprocess/include/process/http.hpp (lines 414 - 417)


Rather than adding this to try to cope with the casting from the enum, 
let's avoid the enum per my suggestion above and remove this helper.



3rdparty/libprocess/src/decoder.hpp (line 439)


newline after this?



3rdparty/libprocess/src/decoder.hpp (line 441)


Can you put this above the if, or just remove it?



3rdparty/libprocess/src/http.cpp (line 76)


We avoid static non-POD types due to destruction issues. Can you put this 
on the heap?

Per my suggestion above, perhaps we can have `Status::string(uint16_t 
code)` as a static function on the `Status` class that provides the string 
version of the status.

This should clean up the tests as well.


- Ben Mahler


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/decoder.hpp 
> 53b8079968a0145651bad9d11aa4be2b504de57a 
>   3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 59-60
> > 
> >
> > How about just status()?
> > 
> > We can also avoid Option here by just returning the number value with 
> > no message? That seems cleaner?
> 
> Timothy Chen wrote:
> Not sure how this is useful if it only returns the number value?
> 
> Ben Mahler wrote:
> Well, you have unconditional gets which can crash the program:
> 
> ```
>   Response(StatusCode _code)
> : type(NONE), code(_code)
>   {
> status = statusString((uint16_t)code).get(); // XXX
>   }
> 
>   explicit Response(
>   const std::string& _body,
>   StatusCode _code)
> : type(BODY),
>   body(_body),
>   code(_code)
>   {
> headers["Content-Length"] = stringify(body.size());
> status = statusString((uint16_t)code).get(); // XXX
>   }
> ```
> 
> Also the tests have unconditional gets, which is unfortunate. Rather than 
> updating all the call sites to deal with None, I would suggest following a 
> similar approach to strerror (which always returns a string, if I pass  I 
> get back "Unknown error: ".
> 
> The other reason is that since the strings coming out from this method 
> include the number (" "), a better answer for unknown 
> statuses is to only include the number (""), since this is already 
> captures that  is omitted. Thoughts?
> 
> Timothy Chen wrote:
> Ah ok I didn't get the default error message part. I think if we're going 
> to keep the map and also use it to get status strings, I think I'll keep the 
> old behavior where we have statuses map which is statically initialized, and 
> just use that to check and get status strings for now. Since we have to check 
> the status code in the map anyway to set the response->flag.

I meant to say I didn't get the default error message part before you left the 
comment :)


- Timothy


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 59-60
> > 
> >
> > How about just status()?
> > 
> > We can also avoid Option here by just returning the number value with 
> > no message? That seems cleaner?
> 
> Timothy Chen wrote:
> Not sure how this is useful if it only returns the number value?

Well, you have unconditional gets which can crash the program:

```
  Response(StatusCode _code)
: type(NONE), code(_code)
  {
status = statusString((uint16_t)code).get(); // XXX
  }

  explicit Response(
  const std::string& _body,
  StatusCode _code)
: type(BODY),
  body(_body),
  code(_code)
  {
headers["Content-Length"] = stringify(body.size());
status = statusString((uint16_t)code).get(); // XXX
  }
```

Also the tests have unconditional gets, which is unfortunate. Rather than 
updating all the call sites to deal with None, I would suggest following a 
similar approach to strerror (which always returns a string, if I pass  I 
get back "Unknown error: ".

The other reason is that since the strings coming out from this method include 
the number (" "), a better answer for unknown statuses is to 
only include the number (""), since this is already captures that 
 is omitted. Thoughts?


- Ben


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 59-60
> > 
> >
> > How about just status()?
> > 
> > We can also avoid Option here by just returning the number value with 
> > no message? That seems cleaner?
> 
> Timothy Chen wrote:
> Not sure how this is useful if it only returns the number value?
> 
> Ben Mahler wrote:
> Well, you have unconditional gets which can crash the program:
> 
> ```
>   Response(StatusCode _code)
> : type(NONE), code(_code)
>   {
> status = statusString((uint16_t)code).get(); // XXX
>   }
> 
>   explicit Response(
>   const std::string& _body,
>   StatusCode _code)
> : type(BODY),
>   body(_body),
>   code(_code)
>   {
> headers["Content-Length"] = stringify(body.size());
> status = statusString((uint16_t)code).get(); // XXX
>   }
> ```
> 
> Also the tests have unconditional gets, which is unfortunate. Rather than 
> updating all the call sites to deal with None, I would suggest following a 
> similar approach to strerror (which always returns a string, if I pass  I 
> get back "Unknown error: ".
> 
> The other reason is that since the strings coming out from this method 
> include the number (" "), a better answer for unknown 
> statuses is to only include the number (""), since this is already 
> captures that  is omitted. Thoughts?

Ah ok I didn't get the default error message part. I think if we're going to 
keep the map and also use it to get status strings, I think I'll keep the old 
behavior where we have statuses map which is statically initialized, and just 
use that to check and get status strings for now. Since we have to check the 
status code in the map anyway to set the response->flag.


- Timothy


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 59-60
> > 
> >
> > How about just status()?
> > 
> > We can also avoid Option here by just returning the number value with 
> > no message? That seems cleaner?

Not sure how this is useful if it only returns the number value?


- Timothy


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 75-160
> > 
> >
> > Whoa, instead of the giant switch can we keep the map, but avoid the 
> > duplicate information?
> > 
> > E.g.
> > 
> > ```
> > statuses[StatusCode::OK] = "200 OK"
> > ```
> > 
> > Possibly even avoiding the need to write 200 here, e.g.
> > 
> > ```
> > statuses[StatusCode::OK] = stringify(StatusCode::OK) + " OK"
> > ```

IMO if all accesses to status code check is just accessing this method, not 
sure what's the benefit of keeping the map.
I can change it back to the map.


- Timothy


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 75-160
> > 
> >
> > Whoa, instead of the giant switch can we keep the map, but avoid the 
> > duplicate information?
> > 
> > E.g.
> > 
> > ```
> > statuses[StatusCode::OK] = "200 OK"
> > ```
> > 
> > Possibly even avoiding the need to write 200 here, e.g.
> > 
> > ```
> > statuses[StatusCode::OK] = stringify(StatusCode::OK) + " OK"
> > ```
> 
> Timothy Chen wrote:
> IMO if all accesses to status code check is just accessing this method, 
> not sure what's the benefit of keeping the map.
> I can change it back to the map.

Just to share the motivation, a map seems to fit the problem better: we have to 
look up information for the status code. We've typically used switches for 
differing control flow / logic as opposed to doing lookup, but it depends on 
the situation. In this case, it just seems nice to split the data (map) from 
the control flow (can we find it in the map?), given the size of the data we're 
dealing with.

I imagine you're resistant to the map because of the initialization function it 
currently requires? How about we eliminate that by keeping a static heap 
allocated map that is initialized using an initializer list?

Also, do you need the explicit casting here?


- Ben


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Oct. 1, 2015, 7:40 p.m., Marco Massenzio wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 75-160
> > 
> >
> > +1 for the map
> > 
> > I am also wondering if it can be templatized or something? maybe 
> > augmenting the `StatusCode` class?
> > 
> > (I've always felt that we should split the `code` -eg, 200; an `int`- 
> > and the `reason` -`"OK"`; a `string`) - it makes comparing "classes" of 
> > responses much easier.
> > 
> > For example, in Python's `requests` this is what one can typically do:
> > ```
> > r = req.get('http://google.com')
> > if 200 <= r.status_code < 300:
> > # all is well
> > elif r.status_code >= 400:
> > # yuk! we hit the skids
> > ```

It's going to be an int as Ben suggested. About the status string, we currently 
just use the status string directly to assign to HTTP responses, so we could 
modify that we always return a code + status string. But I think until we see a 
use case where we really just want the status text portion I'll leave it as is.


- Timothy


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Marco Massenzio]

> On Oct. 1, 2015, 7:40 p.m., Marco Massenzio wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 75-160
> > 
> >
> > +1 for the map
> > 
> > I am also wondering if it can be templatized or something? maybe 
> > augmenting the `StatusCode` class?
> > 
> > (I've always felt that we should split the `code` -eg, 200; an `int`- 
> > and the `reason` -`"OK"`; a `string`) - it makes comparing "classes" of 
> > responses much easier.
> > 
> > For example, in Python's `requests` this is what one can typically do:
> > ```
> > r = req.get('http://google.com')
> > if 200 <= r.status_code < 300:
> > # all is well
> > elif r.status_code >= 400:
> > # yuk! we hit the skids
> > ```
> 
> Timothy Chen wrote:
> It's going to be an int as Ben suggested. About the status string, we 
> currently just use the status string directly to assign to HTTP responses, so 
> we could modify that we always return a code + status string. But I think 
> until we see a use case where we really just want the status text portion 
> I'll leave it as is.

Ok - whatever we return, let's make sure we are consistent with the RFC (2616) 
so that we don't confuse client libraries: https://www.ietf.org/rfc/rfc2616.txt
(see section 6.1):
```
   Status-Line = HTTP-Version SP Status-Code SP Reason-Phrase CRLF
```

(and I'm sorry, I'm missing context here, so this may not be applicable).


- Marco


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38416]

Failed command: ./support/apply-review.sh -n -r 38416

Error:
 2015-09-25 17:48:15 URL:https://reviews.apache.org/r/38416/diff/raw/ 
[27994/27994] -> "38416.patch" [1]
Successfully applied: Allow HTTP response codes to be checked with code.

Allow HTTP response codes to be checked with code.


Review: https://reviews.apache.org/r/38416
Checking 8 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  src/scheduler/scheduler.cpp
libprocess:
  3rdparty/libprocess/include/process/http.hpp
  3rdparty/libprocess/src/decoder.hpp
  3rdparty/libprocess/src/http.cpp
  3rdparty/libprocess/src/process.cpp
  3rdparty/libprocess/src/tests/benchmarks.cpp
  3rdparty/libprocess/src/tests/http_tests.cpp
  3rdparty/libprocess/src/tests/process_tests.cpp
Failed to commit patch

- Mesos ReviewBot


On Sept. 25, 2015, 4:50 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 4:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
>   src/scheduler/scheduler.cpp ee146eb81a8ad72bc04c62d1e223de6aa27b351d 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Sept. 25, 2015, 4:50 p.m.)


Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang Yan 
Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/decoder.hpp 67a5135f302153e376e8dfe8db82aa0b15449389 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
  3rdparty/libprocess/src/process.cpp 4afa30569b4d235637b49a624602e6b199c32e0e 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 
  3rdparty/libprocess/src/tests/process_tests.cpp 
1023500ac2e3c55be955c4686e7f720eba6b4cc9 
  src/scheduler/scheduler.cpp ee146eb81a8ad72bc04c62d1e223de6aa27b351d 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 25, 2015, 5:48 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [38416]
> > 
> > Failed command: ./support/apply-review.sh -n -r 38416
> > 
> > Error:
> >  2015-09-25 17:48:15 URL:https://reviews.apache.org/r/38416/diff/raw/ 
> > [27994/27994] -> "38416.patch" [1]
> > Successfully applied: Allow HTTP response codes to be checked with code.
> > 
> > Allow HTTP response codes to be checked with code.
> > 
> > 
> > Review: https://reviews.apache.org/r/38416
> > Checking 8 files using filter 
> > --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
> > Total errors found: 0
> > ERROR: Commit spanning multiple projects.
> > 
> > Please use separate commits for mesos, libprocess and stout.
> > 
> > Paths grouped by project:
> > mesos:
> >   src/scheduler/scheduler.cpp
> > libprocess:
> >   3rdparty/libprocess/include/process/http.hpp
> >   3rdparty/libprocess/src/decoder.hpp
> >   3rdparty/libprocess/src/http.cpp
> >   3rdparty/libprocess/src/process.cpp
> >   3rdparty/libprocess/src/tests/benchmarks.cpp
> >   3rdparty/libprocess/src/tests/http_tests.cpp
> >   3rdparty/libprocess/src/tests/process_tests.cpp
> > Failed to commit patch

I'll split the patch when I commit, please review the current code.


- Timothy


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


On Sept. 25, 2015, 4:50 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 4:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
>   src/scheduler/scheduler.cpp ee146eb81a8ad72bc04c62d1e223de6aa27b351d 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 23, 2015, 4:58 p.m., Anand Mazumdar wrote:
> > 3rdparty/libprocess/src/http.cpp, line 77
> > 
> >
> > Why not implement a `stringify(...)` for the newly created enum and 
> > make the `statuses[]` use it ? This would allow us to now keep two things 
> > at sync at once unless I am missing something ?
> 
> Anand Mazumdar wrote:
> Thinking about this again let's scope this patch to not cleaning up 
> statuses for now. We can take it up as a separate issue. I am dropping this 
> issue. Let me know if you feel otherwise.

This is fixed in this patch now as well.


- Timothy


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


On Sept. 25, 2015, 4:50 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 4:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
>   src/scheduler/scheduler.cpp ee146eb81a8ad72bc04c62d1e223de6aa27b351d 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Sept. 25, 2015, 10:38 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/decoder.hpp 67a5135f302153e376e8dfe8db82aa0b15449389 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
  3rdparty/libprocess/src/process.cpp 4afa30569b4d235637b49a624602e6b199c32e0e 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 
  3rdparty/libprocess/src/tests/process_tests.cpp 
1023500ac2e3c55be955c4686e7f720eba6b4cc9 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

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



3rdparty/libprocess/include/process/http.hpp (lines 58 - 59)


How about just status()?

We can also avoid Option here by just returning the number value with no 
message? That seems cleaner?



3rdparty/libprocess/include/process/http.hpp (line 396)


Seems unintuitive to use the StatusCode enum type here, given the value may 
lie outside the set of enum values?



3rdparty/libprocess/src/http.cpp (lines 72 - 157)


Whoa, instead of the giant switch can we keep the map, but avoid the 
duplicate information?

E.g.

```
statuses[StatusCode::OK] = "200 OK"
```

Possibly even avoiding the need to write 200 here, e.g.

```
statuses[StatusCode::OK] = stringify(StatusCode::OK) + " OK"
```


- Ben Mahler


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 1023500ac2e3c55be955c4686e7f720eba6b4cc9 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Anand Mazumdar]

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


LGTM , just some comments (see inline) around the `struct Status` being an 
`enum` directly and `int code` being an `Status code` too. We can do a 
`static_cast` in the decoder for converting the `int` status code to `enum` 
i.e. `decoder->response->code = 
static_cast(decoder->parser.status_code);` What do you think ?


3rdparty/libprocess/include/process/http.hpp (line 94)


Why not `enum class Status` directly from C++11 ?



3rdparty/libprocess/include/process/http.hpp (line 391)


Read my earlier comment about `struct Status` being `enum class Status` and 
this can be changed to `Status code` ?

I hate `enum to int` conversions anyways :(



3rdparty/libprocess/src/http.cpp (line 77)


Why not implement a `stringify(...)` for the newly created enum and make 
the `statuses[]` use it ? This would allow us to now keep two things at sync at 
once unless I am missing something ?


- Anand Mazumdar


On Sept. 23, 2015, 7:59 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 23, 2015, 7:59 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Anand Mazumdar]

> On Sept. 23, 2015, 4:58 p.m., Anand Mazumdar wrote:
> > 3rdparty/libprocess/src/http.cpp, line 77
> > 
> >
> > Why not implement a `stringify(...)` for the newly created enum and 
> > make the `statuses[]` use it ? This would allow us to now keep two things 
> > at sync at once unless I am missing something ?

Thinking about this again let's scope this patch to not cleaning up statuses 
for now. We can take it up as a separate issue. I am dropping this issue. Let 
me know if you feel otherwise.


- Anand


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


On Sept. 23, 2015, 7:59 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 23, 2015, 7:59 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

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



3rdparty/libprocess/include/process/http.hpp (lines 657 - 658)


It seems odd to have the code duplicated above, can you set the status 
using the code?

E.g.

```
code = Status::SERVICE_UNAVAILABLE;
status = stringify(code)+ " Service Unavailable";
```

Or why don't we leverage the statuses map here? We have redundant 
information =/


- Ben Mahler


On Sept. 23, 2015, 7:59 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 23, 2015, 7:59 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38416]

All tests passed.

- Mesos ReviewBot


On Sept. 23, 2015, 7:59 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 23, 2015, 7:59 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Sept. 23, 2015, 7:59 a.m.)


Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang Yan 
Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/decoder.hpp 67a5135f302153e376e8dfe8db82aa0b15449389 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Isabel Jimenez]

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

Ship it!


Ship It!

- Isabel Jimenez


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Gilbert Song]

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

Ship it!


This improvement brings convenience.

- Gilbert Song


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 15, 2015, 6:03 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 15, 2015, 6:03 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote:
> > Looks like you're missing the change to decoder. On that note, what happens 
> > when we receive a response with a code that we haven't added to our enum?

You're right I missed the decoding part! Let me populate all the codes in the 
enum and set it correctly.


- Timothy


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

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


Looks like you're missing the change to decoder. On that note, what happens 
when we receive a response with a code that we haven't added to our enum?

- Ben Mahler


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote:
> > Looks like you're missing the change to decoder. On that note, what happens 
> > when we receive a response with a code that we haven't added to our enum?
> 
> Timothy Chen wrote:
> You're right I missed the decoding part! Let me populate all the codes in 
> the enum and set it correctly.
> 
> Ben Mahler wrote:
> Keep in mind that we still have to handle codes we don't have in our enum 
> (some are non-standard, we might receive something random, etc).

definitely, the existing code that checks the http map will still exist.


- Timothy


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Guangya Liu]

> On Sept. 16, 2015, 1:43 a.m., Guangya Liu wrote:
> >
> 
> Timothy Chen wrote:
> it looks like the structs are ordered by response code, so just moving 
> this to the right place assuming so

Got it, this can make the code clear. Thanks.


- Guangya


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

> On Sept. 16, 2015, 1:43 a.m., Guangya Liu wrote:
> >

it looks like the structs are ordered by response code, so just moving this to 
the right place assuming so


- Timothy


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38416]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Guangya Liu]

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



3rdparty/libprocess/include/process/http.hpp (lines 559 - 584)


Just curious why the order of the response are changed?


- Guangya Liu


On 九月 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated 九月 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>