You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2016/12/03 02:14:50 UTC
Review Request 54333: Fixed error messages to say "form body" rather
than "query parameter".
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54333/
-----------------------------------------------------------
Review request for mesos and Anand Mazumdar.
Repository: mesos
Description
-------
I've been told that since we're looking at the request __body__
for the parameters as opposed to the URL, we should report that there
is an error in the "form body" as opposed to a "query parameter".
Diffs
-----
src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f
Diff: https://reviews.apache.org/r/54333/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 54333: Fixed error messages to say "form body"
rather than "query parameter".
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54333/#review159349
-----------------------------------------------------------
src/master/http.cpp (line 996)
<https://reviews.apache.org/r/54333/#comment230312>
Had the `Content-Type` been `application/x-www-form-urlencoded` or other form related content type, it would have made sense to have the "form body" suffix. How about:
```
Missing 'slaveId' query parameter in the request body
```
Ditto for all the other occurences. In hindsight, it won't have been a bad idea to pass them in the URL itself initially. The opinion though on it are pretty divided when using `POST`. :-)
- Anand Mazumdar
On Dec. 3, 2016, 2:14 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54333/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2016, 2:14 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> I've been told that since we're looking at the request __body__
> for the parameters as opposed to the URL, we should report that there
> is an error in the "form body" as opposed to a "query parameter".
>
>
> Diffs
> -----
>
> src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f
>
> Diff: https://reviews.apache.org/r/54333/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 54333: Fixed error messages to say "form body"
rather than "query parameter".
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54333/#review157894
-----------------------------------------------------------
Patch looks great!
Reviews applied: [54333]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 3, 2016, 2:14 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54333/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2016, 2:14 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> I've been told that since we're looking at the request __body__
> for the parameters as opposed to the URL, we should report that there
> is an error in the "form body" as opposed to a "query parameter".
>
>
> Diffs
> -----
>
> src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f
>
> Diff: https://reviews.apache.org/r/54333/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 54333: Specified that we search for the query
parameter in the request body.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54333/#review159354
-----------------------------------------------------------
Ship it!
Ship It!
- Anand Mazumdar
On Dec. 15, 2016, 7:16 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54333/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2016, 7:16 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since we're looking at the request body for the query string as opposed
> to the URL, we should be more specific in reporting that there is an
> error in the "request body".
>
>
> Diffs
> -----
>
> src/master/http.cpp d52806dcf8e4d64ebb98e191a01408c0fcae17ac
>
> Diff: https://reviews.apache.org/r/54333/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 54333: Specified that we search for the query
parameter in the request body.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54333/
-----------------------------------------------------------
(Updated Dec. 15, 2016, 11:16 a.m.)
Review request for mesos and Anand Mazumdar.
Changes
-------
Addressed Anand's comment.
Summary (updated)
-----------------
Specified that we search for the query parameter in the request body.
Repository: mesos
Description (updated)
-------
Since we're looking at the request body for the query string as opposed
to the URL, we should be more specific in reporting that there is an
error in the "request body".
Diffs (updated)
-----
src/master/http.cpp d52806dcf8e4d64ebb98e191a01408c0fcae17ac
Diff: https://reviews.apache.org/r/54333/diff/
Testing
-------
Thanks,
Michael Park