----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32982/#review83674 -----------------------------------------------------------
Ship it! This looks really good - it's great to have documentation! Minor comments about the API details. docs/reservation.md <https://reviews.apache.org/r/32982/#comment134693> this seems to imply that in the Request, the `slave_id` is some part of a "form" submission: ``` -d, --data <data> (HTTP) Sends the specified data in a POST request to the HTTP server, in the same way that a browser does when a user has filled in an HTML form and presses the submit button. This will cause curl to pass the data to the server using the content-type application/x-www-form-urlencoded. ``` IMO the API should instead carry the `slave_id` either as part of the URL (`/reserve/slave_id/1234-xyz`) or part of the JSON body: ``` { "slave_id": "1234-xyz", "resources": { "name" : "cpus", "type" : "SCALAR", "scalar" : { "value" : 8 }, ... } } ``` Also, we need to be sure that the `Content-Type` is set accordingly (`application/json`) The same for all the other endpoints. docs/reservation.md <https://reviews.apache.org/r/32982/#comment134694> Let us please add at least a suggestion of the possible error codes (and a couple Response bodies) here too - at least people can comment. Here's my suggestion: ``` 400 - something went wrong, err_msg in JSON 401 - the role/framework is not authorized to reserve stuff 403 - the configuration and/or this slave do not allow reserved resources 409 - you can't do this; for example: I offered you 4 CPUs and you want to reserve 8 ``` and our all-time favorite: ``` 404 - the resources you want to reserve cannot be found ("name": "cash", "value": "$2MM") ``` Only half-joking here: I can't think of any situation where a request like this could give a 404, but maybe there are some? My recommendation would be to have the codes here (and for `unreserve` too) so that people can comment. - Marco Massenzio On May 13, 2015, 10:15 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32982/ > ----------------------------------------------------------- > > (Updated May 13, 2015, 10:15 p.m.) > > > Review request for mesos, Alexander Rukletsov, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-2205 > https://issues.apache.org/jira/browse/MESOS-2205 > > > Repository: mesos > > > Description > ------- > > See summary. > > NOTE: The framework API should be reviewed thoroughly at this point since > those have been implemented and landed. > The master API endpoints however are still in development and therefore > the state reflected in this patch is incomplete. (e.g. what HTTP code do we > return on failures?) > I'll update the guide to accurately reflect the master endpoints as it > gets more solified. > The purpose of including it was/is to get initial feedback on how it > generally works. > > > Diffs > ----- > > docs/reservation.md PRE-CREATION > > Diff: https://reviews.apache.org/r/32982/diff/ > > > Testing > ------- > > Documentation. > > > Thanks, > > Michael Park > >
