You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Yongqiao Wang <yq...@cn.ibm.com> on 2016/01/05 12:51:57 UTC
Re: Review Request 41789: Expose the http::internal::request function.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 11:51 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Expose the http::internal::request function in the header and not add an additional method/overload for put function.
Summary (updated)
-----------------
Expose the http::internal::request function.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description (updated)
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
> On Jan. 11, 2016, 8:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1218-1221
> > <https://reviews.apache.org/r/41789/diff/11/?file=1189249#file1189249line1218>
> >
> > Will you add `query` as a parameter only once you've moved `get()` over to this?
> >
> > Does anybody even set the `contentType` yet?
>
> Yongqiao Wang wrote:
> Yes, I planed to add query parametner once moved get() method. I will log another JIRA to clear the get() and post() method.
>
> And I only found contentType is set in master_maintenance_tests.cpp:
> // Initialize the default POST header.
> headers["Content-Type"] = "application/json";
>
> Adam B wrote:
> Sounds good. File the JIRA for get/post and then we can close this review issue.
>
> Not sure if we need contentType (yet) if it's always "application/json". No harm in leaving it in, I guess. What do you think?
For the clean up, I have created a new ticket https://issues.apache.org/jira/browse/MESOS-4440, can we fix this later and commit this change first?
- Yongqiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review113694
-----------------------------------------------------------
On Jan. 18, 2016, 6:48 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2016, 6:48 a.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
> 3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
> On Jan. 11, 2016, 8:12 a.m., Adam B wrote:
> > Why not add an overload for `put()`? It could call into the more generic `request()`, like you want to do with post/get
For enhancing the libprocess/http to send put request besides get and post requests, there already has a JIRA for this, refer https://issues.apache.org/jira/browse/MESOS-3763 for details. But in that JIRA, @BenJamin Mahler and @Anand Mazumdar are all suggest to expose internal::http::request as a general function in the header and not add an additional method/overload for put function, I think it is make sence.
Because dynamic weights test need this fix urgently, so I post this patch to expose internal::http::request.
> On Jan. 11, 2016, 8:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/http.cpp, line 1250
> > <https://reviews.apache.org/r/41789/diff/11/?file=1189249#file1189249line1250>
> >
> > Why not continue to call it 'request'? `http::httpRequest()` seems redundant.
'request' is a key word (the Request variable name) in the post()/get() method, for reducing the code modification, so I rename it to httpRequest. Can we change it back to 'reqeust' in the JIRA to clear get() and post() method?
> On Jan. 11, 2016, 8:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1218-1221
> > <https://reviews.apache.org/r/41789/diff/11/?file=1189249#file1189249line1218>
> >
> > Will you add `query` as a parameter only once you've moved `get()` over to this?
> >
> > Does anybody even set the `contentType` yet?
Yes, I planed to add query parametner once moved get() method. I will log another JIRA to clear the get() and post() method.
And I only found contentType is set in master_maintenance_tests.cpp:
// Initialize the default POST header.
headers["Content-Type"] = "application/json";
- Yongqiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review113694
-----------------------------------------------------------
On Jan. 9, 2016, 11:37 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2016, 11:37 a.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
> 3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Adam B <ad...@mesosphere.io>.
> On Jan. 11, 2016, 12:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1218-1221
> > <https://reviews.apache.org/r/41789/diff/11/?file=1189249#file1189249line1218>
> >
> > Will you add `query` as a parameter only once you've moved `get()` over to this?
> >
> > Does anybody even set the `contentType` yet?
>
> Yongqiao Wang wrote:
> Yes, I planed to add query parametner once moved get() method. I will log another JIRA to clear the get() and post() method.
>
> And I only found contentType is set in master_maintenance_tests.cpp:
> // Initialize the default POST header.
> headers["Content-Type"] = "application/json";
Sounds good. File the JIRA for get/post and then we can close this review issue.
Not sure if we need contentType (yet) if it's always "application/json". No harm in leaving it in, I guess. What do you think?
> On Jan. 11, 2016, 12:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/http.cpp, line 1250
> > <https://reviews.apache.org/r/41789/diff/11/?file=1189249#file1189249line1250>
> >
> > Why not continue to call it 'request'? `http::httpRequest()` seems redundant.
>
> Yongqiao Wang wrote:
> 'request' is a key word (the Request variable name) in the post()/get() method, for reducing the code modification, so I rename it to httpRequest. Can we change it back to 'reqeust' in the JIRA to clear get() and post() method?
I still hate seeing `http::httpRequest(...)` since it violates the Mesos style of avoiding redundancies. `http::request()` should be sufficiently scoped in callers to prevent confusing it with a `request` variable. If you're worried about the confusion within the function, then you can rename the parameter to `_request`, as recommended by http://mesos.apache.org/documentation/latest/c++-style-guide/
"We prepend constructor and function arguments with a leading underscore to avoid ambiguity and/or shadowing"
Callers could still use `request` for their local variables and `http::request()` for the function, as those can be easily scoped. Then you would only have to rename `request` to `_request` inside your new `http::request()` function, not across all callers.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review113694
-----------------------------------------------------------
On Jan. 17, 2016, 10:48 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2016, 10:48 p.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
> 3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Adam B <ad...@mesosphere.io>.
> On Jan. 11, 2016, 12:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1218-1221
> > <https://reviews.apache.org/r/41789/diff/11/?file=1189249#file1189249line1218>
> >
> > Will you add `query` as a parameter only once you've moved `get()` over to this?
> >
> > Does anybody even set the `contentType` yet?
>
> Yongqiao Wang wrote:
> Yes, I planed to add query parametner once moved get() method. I will log another JIRA to clear the get() and post() method.
>
> And I only found contentType is set in master_maintenance_tests.cpp:
> // Initialize the default POST header.
> headers["Content-Type"] = "application/json";
>
> Adam B wrote:
> Sounds good. File the JIRA for get/post and then we can close this review issue.
>
> Not sure if we need contentType (yet) if it's always "application/json". No harm in leaving it in, I guess. What do you think?
>
> Yongqiao Wang wrote:
> For the clean up, I have created a new ticket https://issues.apache.org/jira/browse/MESOS-4440, can we fix this later and commit this change first?
Looks good, thanks. Marking this review issue as resolved. We can worry about MESOS-4440 after we commit the rest of this.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review113694
-----------------------------------------------------------
On Jan. 17, 2016, 10:48 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2016, 10:48 p.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
> 3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review113694
-----------------------------------------------------------
Why not add an overload for `put()`? It could call into the more generic `request()`, like you want to do with post/get
3rdparty/libprocess/include/process/http.hpp (lines 802 - 804)
<https://reviews.apache.org/r/41789/#comment174487>
This would all fit on one line, so please unwrap.
3rdparty/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/41789/#comment174488>
Please add this line back.
3rdparty/libprocess/src/http.cpp (lines 1218 - 1221)
<https://reviews.apache.org/r/41789/#comment174489>
Will you add `query` as a parameter only once you've moved `get()` over to this?
Does anybody even set the `contentType` yet?
3rdparty/libprocess/src/http.cpp (line 1223)
<https://reviews.apache.org/r/41789/#comment174491>
What if SSL is enabled/required?
How would you use https?
3rdparty/libprocess/src/http.cpp (line 1250)
<https://reviews.apache.org/r/41789/#comment174490>
Why not continue to call it 'request'? `http::httpRequest()` seems redundant.
- Adam B
On Jan. 9, 2016, 3:37 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2016, 3:37 a.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
> 3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Adam B <ad...@mesosphere.io>.
> On Jan. 19, 2016, 7:11 p.m., Adam B wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 812-814
> > <https://reviews.apache.org/r/41789/diff/11-17/?file=1189248#file1189248line812>
> >
> > Seems like a good candidate for a doxygen-style @param
>
> Yongqiao Wang wrote:
> I think @param is for /* */ style. In // style i find some guys use '', you can find another one in https://github.com/apache/mesos/blob/master/src/master/master.hpp#L560
We're slowly trying to move the entire code base to `/* */` doxygen style. The line you reference is from May 2015, but you'll notice that newer functions in master.hpp and even in http.hpp use the new doxygen style.
See http://mesos.apache.org/documentation/latest/doxygen-style-guide/
"This guide introduces a consistent style for documenting Mesos source code using Doxygen. There is an ongoing, incremental effort with the goal to document all public Mesos, libprocess, and stout APIs this way. For now, existing code may not follow these guidelines, but new code should."
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review115326
-----------------------------------------------------------
On Jan. 17, 2016, 10:48 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2016, 10:48 p.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
> 3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
> On Jan. 20, 2016, 3:11 a.m., Adam B wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 812-814
> > <https://reviews.apache.org/r/41789/diff/11-17/?file=1189248#file1189248line812>
> >
> > Seems like a good candidate for a doxygen-style @param
I think @param is for /* */ style. In // style i find some guys use '', you can find another one in https://github.com/apache/mesos/blob/master/src/master/master.hpp#L560
- Yongqiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review115326
-----------------------------------------------------------
On Jan. 18, 2016, 6:48 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2016, 6:48 a.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
> 3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review115326
-----------------------------------------------------------
Looks good. Besides the previously mentioned `http::request()` name change and the redundant code in `createRequest()`s, this looks shippable.
3rdparty/libprocess/include/process/http.hpp (lines 812 - 814)
<https://reviews.apache.org/r/41789/#comment176293>
Seems like a good candidate for a doxygen-style @param
3rdparty/libprocess/src/http.cpp (lines 1240 - 1268)
<https://reviews.apache.org/r/41789/#comment176295>
Looks like a lot of redundant content between these two `createRequest()` calls. Can one of them call the other, or both of them call a common helper? Let's try to remove code duplication whenever possible.
3rdparty/libprocess/src/http.cpp (line 1277)
<https://reviews.apache.org/r/41789/#comment176294>
s/enableSSL?/enableSSL ?/ since we like whitespace around our operators.
3rdparty/libprocess/src/tests/http_tests.cpp (line 685)
<https://reviews.apache.org/r/41789/#comment176299>
Rename to `validateDeleteHttpRequest` since that's what it's doing, and it clearly won't validate any other `http::Request`s
3rdparty/libprocess/src/tests/http_tests.cpp (line 705)
<https://reviews.apache.org/r/41789/#comment176300>
"Newline when calling or defining a function: indent with 4 spaces."
Although you were correct for the previous line:
"Newline for an assignment statement: indent with 2 spaces."
See http://mesos.apache.org/documentation/latest/c++-style-guide/
- Adam B
On Jan. 17, 2016, 10:48 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2016, 10:48 p.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
> 3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 25, 2016, 3:26 p.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Rebase to address merge error.
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description
-------
Expose the http::internal::request function.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp bcba3040e902b34ef6707fe885595bcf59eb964b
3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0
3rdparty/libprocess/src/tests/http_tests.cpp 66d185e3fd8a165d8a98d3d2752e756f8de3499b
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 21, 2016, 9:56 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Add dependency to address the build error for #41790
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description
-------
Expose the http::internal::request function.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp 1d9a944f7b0c559823c8c889d20e942c7d6a7221
3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0
3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 20, 2016, 8:52 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Addressed comments.
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description
-------
Expose the http::internal::request function.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp 1d9a944f7b0c559823c8c889d20e942c7d6a7221
3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0
3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review115381
-----------------------------------------------------------
Ship it!
Looks great! I'll commit this tomorrow if I don't hear any objection from Joris (the original shepherd of MESOS-3763).
3rdparty/libprocess/include/process/http.hpp (lines 830 - 831)
<https://reviews.apache.org/r/41789/#comment176369>
s/Clean/Refactor/
s/and using/to use/
- Adam B
On Jan. 20, 2016, 12:26 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 12:26 a.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the http::internal::request function.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 1d9a944f7b0c559823c8c889d20e942c7d6a7221
> 3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0
> 3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 20, 2016, 8:26 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Addressed comments of Adam.
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description
-------
Expose the http::internal::request function.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp 1d9a944f7b0c559823c8c889d20e942c7d6a7221
3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0
3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 20, 2016, 8:09 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Address the comments of Adam.
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description (updated)
-------
Expose the http::internal::request function.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp 1d9a944f7b0c559823c8c889d20e942c7d6a7221
3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0
3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 18, 2016, 6:48 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Add test.
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp 1fe549e9c3c64b310048388d90ab04e5641e08a1
3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e
3rdparty/libprocess/src/tests/http_tests.cpp c23d0bf1929686cfc42969f39ce046f4794539d4
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/#review114682
-----------------------------------------------------------
3rdparty/libprocess/src/http.cpp (line 1280)
<https://reviews.apache.org/r/41789/#comment175545>
Would it make sense to add a test (e.g. testing DELETE)?
- Joerg Schad
On Jan. 15, 2016, 7:50 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2016, 7:50 a.m.)
>
>
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
>
>
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Expose the internal::http::request function in the header and not add an additional method/overload for put function.
>
> (TODO): Clean the other instances of post/get to use the http::request method.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp d0f820e0850955c01c5149fef2f05a1becb4bd32
> 3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9
>
> Diff: https://reviews.apache.org/r/41789/diff/
>
>
> Testing
> -------
>
> Make & Make check successfully.
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 14, 2016, 11:50 p.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Changed linked JIRA from MESOS-4200 (dynamic weights tests) to MESOS-3763 (http::put)
Bugs: MESOS-3763
https://issues.apache.org/jira/browse/MESOS-3763
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp d0f820e0850955c01c5149fef2f05a1becb4bd32
3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 7:30 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Rebase.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp d0f820e0850955c01c5149fef2f05a1becb4bd32
3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 4:05 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Add the description for streamedResponse parameter.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp d0f820e0850955c01c5149fef2f05a1becb4bd32
3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 11, 2016, 2:49 p.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Rebase.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 11, 2016, 11:39 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 11, 2016, 11:33 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Addressed the comments of Adam.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 9, 2016, 11:37 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Rebase.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 2:15 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Fix the build error.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 2:07 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Rebase.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 1:54 a.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Add a common function to let caller to generate the Request object.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang
Re: Review Request 41789: Expose the http::internal::request function.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41789/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 1:41 p.m.)
Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil Conway, and Qian Zhang.
Changes
-------
Rebase.
Bugs: MESOS-4200
https://issues.apache.org/jira/browse/MESOS-4200
Repository: mesos
Description
-------
Expose the internal::http::request function in the header and not add an additional method/overload for put function.
(TODO): Clean the other instances of post/get to use the http::request method.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp ed708fe4b0006782a19f9c61603f152e32a02e8e
3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2
Diff: https://reviews.apache.org/r/41789/diff/
Testing
-------
Make & Make check successfully.
Thanks,
Yongqiao Wang