Re: Review Request 36847: Added HTTP Delete Method.

2015-08-14 Thread Bernd Mathiske

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

Ship it!


Ship It!

- Bernd Mathiske


On Aug. 13, 2015, 9:19 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 13, 2015, 9:19 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b829dad9d1c5d98f09a3a3c7cc04e7659f6616a4 
   3rdparty/libprocess/src/http.cpp b70e2fa0f500216c15f20907816874b9486826b8 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 7351fa44e69743ab82e9cada7ba7f797a6899bab 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36847]

All tests passed.

- Mesos ReviewBot


On Aug. 13, 2015, 4:19 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 13, 2015, 4:19 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b829dad9d1c5d98f09a3a3c7cc04e7659f6616a4 
   3rdparty/libprocess/src/http.cpp b70e2fa0f500216c15f20907816874b9486826b8 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 7351fa44e69743ab82e9cada7ba7f797a6899bab 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-13 Thread Joerg Schad


 On Aug. 6, 2015, 9:56 a.m., Alexander Rojas wrote:
  3rdparty/libprocess/include/process/http.hpp, line 754
  https://reviews.apache.org/r/36847/diff/7/?file=1028524#file1028524line754
 
  I'm rather late to the party, but AFAIK Mesos appreciates consistency 
  over doing some things right and some wrong.
  
  The Mesos way would be to add the method in a consistent way and then 
  do a sweep change for all the other http calls.
  
  That being said, the way I have seen other http libraries getting 
  around the keyword issue is by using an underscore at some point.
 
 Marco Massenzio wrote:
 To paraphrase a well-known maxim... I'd rather be sometimes right, than 
 consistently wrong :)
 (and, as per my usual, adding here my favorite 
 [demotivator](http://despair.com/collections/demotivators/products/consistency)
  :D
 
 More seriously, good point about underscore, the problem here is that 
 wherever you put it, you end up violating a style rule and/or doing something 
 ugly - a real conumdrum!
 
 Bernd Mathiske wrote:
 There is NO consistent way to add the method and it MUST be added. 
 Therefore I prefer a way that is, halas, inconsistent, but at least not wrong 
 when regarded in isolation in any additional ways. Then, as an additional 
 issue/patch we may/should choose to bring the other calls into line. (An 
 underscore does not achieve any of these objectives.)
 
 Bernd Mathiske wrote:
 All that said, I propose that we choose this name for the method: 
 requestDelete. Reasons:
 - Avoids keyword problem.
 - Is short enough to be applied to all other HTTP requests: requestGet, 
 requestAuth, etc.; clarifies what these do.
 - Reads as a verb.
 - Avoids send as part of the name, which is kinda wrong, since we do 
 more than just sending. We also wait for the response via a future.
 
 Marco Massenzio wrote:
 +1

I added MESOS-3256 to track the renaming effort.


- Joerg


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


On Aug. 4, 2015, 10:57 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 4, 2015, 10:57 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-12 Thread Bernd Mathiske


 On Aug. 6, 2015, 2:56 a.m., Alexander Rojas wrote:
  3rdparty/libprocess/include/process/http.hpp, line 754
  https://reviews.apache.org/r/36847/diff/7/?file=1028524#file1028524line754
 
  I'm rather late to the party, but AFAIK Mesos appreciates consistency 
  over doing some things right and some wrong.
  
  The Mesos way would be to add the method in a consistent way and then 
  do a sweep change for all the other http calls.
  
  That being said, the way I have seen other http libraries getting 
  around the keyword issue is by using an underscore at some point.
 
 Marco Massenzio wrote:
 To paraphrase a well-known maxim... I'd rather be sometimes right, than 
 consistently wrong :)
 (and, as per my usual, adding here my favorite 
 [demotivator](http://despair.com/collections/demotivators/products/consistency)
  :D
 
 More seriously, good point about underscore, the problem here is that 
 wherever you put it, you end up violating a style rule and/or doing something 
 ugly - a real conumdrum!
 
 Bernd Mathiske wrote:
 There is NO consistent way to add the method and it MUST be added. 
 Therefore I prefer a way that is, halas, inconsistent, but at least not wrong 
 when regarded in isolation in any additional ways. Then, as an additional 
 issue/patch we may/should choose to bring the other calls into line. (An 
 underscore does not achieve any of these objectives.)

All that said, I propose that we choose this name for the method: 
requestDelete. Reasons:
- Avoids keyword problem.
- Is short enough to be applied to all other HTTP requests: requestGet, 
requestAuth, etc.; clarifies what these do.
- Reads as a verb.
- Avoids send as part of the name, which is kinda wrong, since we do more 
than just sending. We also wait for the response via a future.


- Bernd


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


On Aug. 4, 2015, 3:57 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 4, 2015, 3:57 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-12 Thread Bernd Mathiske


 On Aug. 6, 2015, 2:56 a.m., Alexander Rojas wrote:
  3rdparty/libprocess/include/process/http.hpp, line 754
  https://reviews.apache.org/r/36847/diff/7/?file=1028524#file1028524line754
 
  I'm rather late to the party, but AFAIK Mesos appreciates consistency 
  over doing some things right and some wrong.
  
  The Mesos way would be to add the method in a consistent way and then 
  do a sweep change for all the other http calls.
  
  That being said, the way I have seen other http libraries getting 
  around the keyword issue is by using an underscore at some point.
 
 Marco Massenzio wrote:
 To paraphrase a well-known maxim... I'd rather be sometimes right, than 
 consistently wrong :)
 (and, as per my usual, adding here my favorite 
 [demotivator](http://despair.com/collections/demotivators/products/consistency)
  :D
 
 More seriously, good point about underscore, the problem here is that 
 wherever you put it, you end up violating a style rule and/or doing something 
 ugly - a real conumdrum!

There is NO consistent way to add the method and it MUST be added. Therefore I 
prefer a way that is, halas, inconsistent, but at least not wrong when regarded 
in isolation in any additional ways. Then, as an additional issue/patch we 
may/should choose to bring the other calls into line. (An underscore does not 
achieve any of these objectives.)


- Bernd


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


On Aug. 4, 2015, 3:57 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 4, 2015, 3:57 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-08 Thread Marco Massenzio


 On Aug. 6, 2015, 9:56 a.m., Alexander Rojas wrote:
  3rdparty/libprocess/include/process/http.hpp, line 754
  https://reviews.apache.org/r/36847/diff/7/?file=1028524#file1028524line754
 
  I'm rather late to the party, but AFAIK Mesos appreciates consistency 
  over doing some things right and some wrong.
  
  The Mesos way would be to add the method in a consistent way and then 
  do a sweep change for all the other http calls.
  
  That being said, the way I have seen other http libraries getting 
  around the keyword issue is by using an underscore at some point.

To paraphrase a well-known maxim... I'd rather be sometimes right, than 
consistently wrong :)
(and, as per my usual, adding here my favorite 
[demotivator](http://despair.com/collections/demotivators/products/consistency) 
:D

More seriously, good point about underscore, the problem here is that wherever 
you put it, you end up violating a style rule and/or doing something ugly - a 
real conumdrum!


- Marco


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


On Aug. 4, 2015, 10:57 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 4, 2015, 10:57 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-06 Thread Alexander Rojas

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



3rdparty/libprocess/include/process/http.hpp (line 754)
https://reviews.apache.org/r/36847/#comment148965

I'm rather late to the party, but AFAIK Mesos appreciates consistency over 
doing some things right and some wrong.

The Mesos way would be to add the method in a consistent way and then do a 
sweep change for all the other http calls.

That being said, the way I have seen other http libraries getting around 
the keyword issue is by using an underscore at some point.


- Alexander Rojas


On Aug. 4, 2015, 12:57 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 4, 2015, 12:57 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-04 Thread Bernd Mathiske

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



3rdparty/libprocess/include/process/http.hpp (line 747)
https://reviews.apache.org/r/36847/#comment148544

This comment does not match the function signature.



3rdparty/libprocess/include/process/http.hpp (line 763)
https://reviews.apache.org/r/36847/#comment148545

s/pid/upid



3rdparty/libprocess/include/process/http.hpp (line 764)
https://reviews.apache.org/r/36847/#comment148546

Please explain what happens if it is none.



3rdparty/libprocess/include/process/http.hpp (line 765)
https://reviews.apache.org/r/36847/#comment148547

s/header/headers



3rdparty/libprocess/src/http.cpp (line 938)
https://reviews.apache.org/r/36847/#comment148548

Because of this concatenation we should explain at the function delcaration 
in http.hpp that it is expected that `path` starts with /.

Or is it not?


- Bernd Mathiske


On Aug. 3, 2015, 11:45 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 3, 2015, 11:45 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-04 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36847]

All tests passed.

- Mesos ReviewBot


On Aug. 4, 2015, 10:57 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 4, 2015, 10:57 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-03 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36847]

All tests passed.

- Mesos ReviewBot


On Aug. 3, 2015, 6:45 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated Aug. 3, 2015, 6:45 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 ecbcbd552ac834659860627c82628ed38e6139b3 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-03 Thread Bernd Mathiske

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



3rdparty/libprocess/include/process/http.hpp (line 736)
https://reviews.apache.org/r/36847/#comment148297

(We have discussed this naming choice at some length offline. Here is how I 
see it.) If I find a call site somewhere that reads http:del(...) I will 
probably wonder what this is and will look it up. Not good enough IMHO. Cryptic.

If instead I read http::sendDeleteRequest(), it is immediately clear to 
me what this call does. 

True, this makes the API unbalanced, since the other calls are simply 
called get and put. Well, they never should have! 

Even if we never rename the other calls, I favor readability at call sites 
over superficially consistent naming across the API.

2c


- Bernd Mathiske


On July 29, 2015, 6:39 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 29, 2015, 6:39 a.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-03 Thread Joerg Schad


 On Aug. 3, 2015, 5:47 p.m., Marco Massenzio wrote:
  3rdparty/libprocess/src/http.cpp, lines 927-929
  https://reviews.apache.org/r/36847/diff/4/?file=1023561#file1023561line927
 
  I would add a statement in the (javadoc?) method's documentation, to 
  the effect that a query or fragment parts should *not* be used in this 
  request, and the outcome of doing so are undefined.
  
  This would also follow the principle of least surprise for the 
  callers of this method (who would also not need to go reverse engineering 
  the code to figure out why their code ain't working...)

I created MESOS-3163 to also handle/discuss this across the other methods. is 
that ok for you?


- Joerg


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


On July 29, 2015, 1:39 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 29, 2015, 1:39 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-03 Thread Joerg Schad

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

(Updated Aug. 3, 2015, 6:45 p.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 
  3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
  3rdparty/libprocess/src/tests/http_tests.cpp 
ecbcbd552ac834659860627c82628ed38e6139b3 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 36847: Added HTTP Delete Method.

2015-08-03 Thread Marco Massenzio

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



3rdparty/libprocess/include/process/http.hpp (line 736)
https://reviews.apache.org/r/36847/#comment148360

please don't use abbreviations.
`delete()` is the HTTP method name.

I'm almost sure it violates our style guide (and I'm positive it violates 
Google's).



3rdparty/libprocess/include/process/http.hpp (line 741)
https://reviews.apache.org/r/36847/#comment148361

I would so much love to see the javadoc used, so this shows up in the 
online documentation (and in my IDE when I hover over the callsite); instead of 
having to go fisthing for the source file and find this documentation



3rdparty/libprocess/src/http.cpp (lines 927 - 929)
https://reviews.apache.org/r/36847/#comment148363

I would add a statement in the (javadoc?) method's documentation, to the 
effect that a query or fragment parts should *not* be used in this request, and 
the outcome of doing so are undefined.

This would also follow the principle of least surprise for the callers of 
this method (who would also not need to go reverse engineering the code to 
figure out why their code ain't working...)


- Marco Massenzio


On July 29, 2015, 1:39 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 29, 2015, 1:39 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-08-03 Thread Joerg Schad


 On Aug. 3, 2015, 5:47 p.m., Marco Massenzio wrote:
  3rdparty/libprocess/include/process/http.hpp, line 736
  https://reviews.apache.org/r/36847/diff/4/?file=1023560#file1023560line736
 
  please don't use abbreviations.
  `delete()` is the HTTP method name.
  
  I'm almost sure it violates our style guide (and I'm positive it 
  violates Google's).

Using delete() unfortunately violates c++ list of reserved keyword 
Previously tried something like sendDeleteRequest which people found to 
long/verbose and inconsistent...


- Joerg


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


On July 29, 2015, 1:39 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 29, 2015, 1:39 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Joerg Schad


 On July 27, 2015, 8:13 p.m., Alexander Rukletsov wrote:
  3rdparty/libprocess/src/http.cpp, line 927
  https://reviews.apache.org/r/36847/diff/1/?file=1022344#file1022344line927
 
  This todo looks inconsistent with the way `GET` handler is written. 
  Let's create a clean-up JIRA ticket.

MESOS-3163


- Joerg


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


On July 28, 2015, 9:49 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 28, 2015, 9:49 a.m.)
 
 
 Review request for mesos and Alexander Rukletsov.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added HTTP Delete Method.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Joerg Schad

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

(Updated July 28, 2015, 9:49 a.m.)


Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Added HTTP Delete Method.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
9faed55247a3ccd629db7b85dbf31d3117e120e9 
  3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
  3rdparty/libprocess/src/tests/http_tests.cpp 
01f243cd9c46e162c16e9bb452a846faf31d1445 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36847]

All tests passed.

- Mesos ReviewBot


On July 28, 2015, 9:49 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 28, 2015, 9:49 a.m.)
 
 
 Review request for mesos and Alexander Rukletsov.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added HTTP Delete Method.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Joerg Schad


 On July 28, 2015, 8:43 a.m., Alexander Rukletsov wrote:
  Also, I think we need a doppelgänger for the method in the `streaming` 
  namespace.

Are you sure? For delete we are not streaming any data...


- Joerg


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


On July 28, 2015, 9:49 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 28, 2015, 9:49 a.m.)
 
 
 Review request for mesos and Alexander Rukletsov.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added HTTP Delete Method.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Alexander Rukletsov


 On July 28, 2015, 8:43 a.m., Alexander Rukletsov wrote:
  Also, I think we need a doppelgänger for the method in the `streaming` 
  namespace.
 
 Joerg Schad wrote:
 Are you sure? For delete we are not streaming any data...

I think we do it implicitly anyway (see `StreamingResponseDecoder`), but I see 
your point, maybe we don't need to expose it for now, we can always do it later 
if needed.


- Alexander


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


On July 28, 2015, 9:49 a.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 28, 2015, 9:49 a.m.)
 
 
 Review request for mesos and Alexander Rukletsov.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added HTTP Delete Method.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Joerg Schad

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

(Updated July 28, 2015, 4:26 p.m.)


Review request for mesos and Alexander Rukletsov.


Summary (updated)
-

Added HTTP Delete Method.


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


Repository: mesos


Description
---

Added HTTP Delete Method.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
9faed55247a3ccd629db7b85dbf31d3117e120e9 
  3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
  3rdparty/libprocess/src/tests/http_tests.cpp 
01f243cd9c46e162c16e9bb452a846faf31d1445 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Alexander Rukletsov

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

Ship it!


Ship It!

- Alexander Rukletsov


On July 28, 2015, 4:26 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 28, 2015, 4:26 p.m.)
 
 
 Review request for mesos and Alexander Rukletsov.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added HTTP Delete Method.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad
 




Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Joerg Schad

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

(Updated July 28, 2015, 4:45 p.m.)


Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description (updated)
---

see summary.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
9faed55247a3ccd629db7b85dbf31d3117e120e9 
  3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
  3rdparty/libprocess/src/tests/http_tests.cpp 
01f243cd9c46e162c16e9bb452a846faf31d1445 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36847]

All tests passed.

- Mesos ReviewBot


On July 28, 2015, 4:45 p.m., Joerg Schad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36847/
 ---
 
 (Updated July 28, 2015, 4:45 p.m.)
 
 
 Review request for mesos and Alexander Rukletsov.
 
 
 Bugs: MESOS-3152
 https://issues.apache.org/jira/browse/MESOS-3152
 
 
 Repository: mesos
 
 
 Description
 ---
 
 see summary.
 
 
 Diffs
 -
 
   3rdparty/libprocess/include/process/http.hpp 
 9faed55247a3ccd629db7b85dbf31d3117e120e9 
   3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 
   3rdparty/libprocess/src/tests/http_tests.cpp 
 01f243cd9c46e162c16e9bb452a846faf31d1445 
 
 Diff: https://reviews.apache.org/r/36847/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Joerg Schad