> On Jan. 17, 2014, 11:52 p.m., Charlie Carson wrote:
> > 3rdparty/libprocess/include/process/http.hpp, line 503
> > <https://reviews.apache.org/r/17076/diff/1/?file=429763#file429763line503>
> >
> >     this is probably a API consistency question and I'm the new guy - but 
> > to me, a post w/o either a body or a contentType is incorrect.  The entire 
> > purpose of a POST is to upload a body and you need to know the content type 
> > to know what to do with the body when it arrives.
> 
> Jie Yu wrote:
>     ContentType is not mandatory as far as I know. The server can infer the 
> ContentType.
>     
>     http://www.w3.org/Protocols/rfc2616/rfc2616-sec7.html#sec7.2.1
>     
>     One can choose to POST an empty request (no body). In that case, 
> ContentType is not needed.
> 
> Charlie Carson wrote:
>     someone up here mention /quitquitquit being a good example of POST w/ no 
> body, so that's definitely a good point.  OTOH, the spec says "Any HTTP/1.1 
> message containing an entity-body SHOULD include a Content-Type header 
> field".  so that makes me want to do two overloads:
>     
>     Future<Response> post(const UPID& upid, const Option<string>& path)
>     Future<Response> post(const UPID& upid, const Option<string>& path, const 
> string& contentType, const string& body)
>     
>     but the second overload won't compile...  the other option would be to 
> have a little struct which represents a entity-body - ex:
>     struct Content {
>       string contentType;
>       string body;
>     }
>     
>     then you could have a single overload for post:
>     
>     Future<Response> post(const UPID& upid, const Option<string>& path, const 
> Option<PostContent>& content)
>     
>     that's nice b/c it would position you for posting things like the 
> contents of a file or a stream (like the HttpResponse does).
>     
>     That said, we are DEFINITELY into deep api / framework style at this 
> point and I'll defer to you.  
>     
>     Feel free to close the issue.
>     
>     
>

Given that the Content-Type isn't necessary I think moving it after the body 
makes sense (since it has a default of 'None()' and is one less parameter that 
would need to get passed). But given that 'path' could also be 'None()' and 
isn't after 'body' I don't think it would be bad to pull 'contentType' before 
'body' to account for the intuitiveness of how it appears on the wire.


> On Jan. 17, 2014, 11:52 p.m., Charlie Carson wrote:
> > 3rdparty/libprocess/src/process.cpp, line 3627
> > <https://reviews.apache.org/r/17076/diff/1/?file=429764#file429764line3627>
> >
> >     what is the rationale for reversing the order of body and contenttype?  
> > I would think we'd want them in order they appear on the wire in the 
> > request so that they are more intuitive.

Yup, let's swap it back.


- Benjamin


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


On Jan. 17, 2014, 11:41 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17076/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 11:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Charlie Carson.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp a8b1636 
>   3rdparty/libprocess/src/process.cpp 936f118 
> 
> Diff: https://reviews.apache.org/r/17076/diff/
> 
> 
> Testing
> -------
> 
> will include test fixes in the following patches.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to