You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <bb...@apache.org> on 2019/11/08 13:13:36 UTC
Review Request 71741: Updated operator API documention to use
rereservation format.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/
-----------------------------------------------------------
Review request for mesos and Benno Evers.
Bugs: MESOS-9993
https://issues.apache.org/jira/browse/MESOS-9993
Repository: mesos
Description
-------
Updated operator API documention to use rereservation format.
Diffs
-----
docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
Diff: https://reviews.apache.org/r/71741/diff/1/
Testing
-------
Previewed in generated site
Thanks,
Benjamin Bannier
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218760
-----------------------------------------------------------
Bad review!
Reviews applied: [71741, 71739, 71725, 71729]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On Nov. 22, 2019, 11:55 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2019, 11:55 a.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/3/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/
-----------------------------------------------------------
(Updated Nov. 22, 2019, 12:55 p.m.)
Review request for mesos and Benno Evers.
Bugs: MESOS-9993
https://issues.apache.org/jira/browse/MESOS-9993
Repository: mesos
Description
-------
Updated operator API documention to use rereservation format.
Diffs (updated)
-----
docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
Diff: https://reviews.apache.org/r/71741/diff/3/
Changes: https://reviews.apache.org/r/71741/diff/2-3/
Testing
-------
Previewed in generated site
Thanks,
Benjamin Bannier
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Nov. 20, 2019, 3:41 p.m., Benno Evers wrote:
> > docs/operator-http-api.md
> > Line 1870 (original), 1872 (patched)
> > <https://reviews.apache.org/r/71741/diff/2/?file=2174609#file2174609line1872>
> >
> > Maybe ask a native speaker for a second opinion, but "takes" sounds a bit weird to me. Maybe "accepts parameters"?
> >
> > Also, I would propose adding a short summary of what the call *does* with these parameters, instead of launching straight into backwards compatibility exceptions, something like
> >
> > ```
> > [...] and the target resources, and updates the reservations of `source` to be equal to the reservations of `target`.
> > ```
`takes` was used by a couple of native speakers in this doc and looks fine to me.
Thanks for the suggestion on describing effects, updated the doc.
> On Nov. 20, 2019, 3:41 p.m., Benno Evers wrote:
> > docs/operator-http-api.md
> > Lines 1956 (patched)
> > <https://reviews.apache.org/r/71741/diff/2/?file=2174609#file2174609line1956>
> >
> > Mentioning "multiple levels" here feels a bit like an implementation detail; from a users perspective we simply allow changing one role to another. (also "levels" are not present in this example anyways.)
Good point. Dropping this whole sentence as we now already already above call out the requirement for `source` and `resources` to be identical apart from reservations.
> On Nov. 20, 2019, 3:41 p.m., Benno Evers wrote:
> > docs/operator-http-api.md
> > Lines 1957 (patched)
> > <https://reviews.apache.org/r/71741/diff/2/?file=2174609#file2174609line1957>
> >
> > s/fall/for/
Sentence was removed.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218721
-----------------------------------------------------------
On Nov. 22, 2019, 12:55 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2019, 12:55 p.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/3/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218721
-----------------------------------------------------------
Fix it, then Ship it!
docs/operator-http-api.md
Lines 1869 (patched)
<https://reviews.apache.org/r/71741/#comment306589>
Maybe `s/change/update/`, since we consistently talk about "reservation updates" throughout the patch series?
docs/operator-http-api.md
Line 1869 (original), 1871 (patched)
<https://reviews.apache.org/r/71741/#comment306585>
s/reserve/reserves/
docs/operator-http-api.md
Line 1870 (original), 1872 (patched)
<https://reviews.apache.org/r/71741/#comment306586>
Maybe ask a native speaker for a second opinion, but "takes" sounds a bit weird to me. Maybe "accepts parameters"?
Also, I would propose adding a short summary of what the call *does* with these parameters, instead of launching straight into backwards compatibility exceptions, something like
```
[...] and the target resources, and updates the reservations of `source` to be equal to the reservations of `target`.
```
docs/operator-http-api.md
Lines 1876 (patched)
<https://reviews.apache.org/r/71741/#comment306587>
Maybe we should also mention here that `source` and `resources` must be identical except for their reservations in this case.
docs/operator-http-api.md
Lines 1956 (patched)
<https://reviews.apache.org/r/71741/#comment306588>
Mentioning "multiple levels" here feels a bit like an implementation detail; from a users perspective we simply allow changing one role to another. (also "levels" are not present in this example anyways.)
docs/operator-http-api.md
Lines 1957 (patched)
<https://reviews.apache.org/r/71741/#comment306584>
s/fall/for/
- Benno Evers
On Nov. 20, 2019, 2:17 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2019, 2:17 p.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/2/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218724
-----------------------------------------------------------
Bad review!
Reviews applied: [71741, 71739, 71725, 71729]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On Nov. 20, 2019, 2:17 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2019, 2:17 p.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/2/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/
-----------------------------------------------------------
(Updated Nov. 20, 2019, 3:17 p.m.)
Review request for mesos and Benno Evers.
Changes
-------
Expand documentation and examples
Bugs: MESOS-9993
https://issues.apache.org/jira/browse/MESOS-9993
Repository: mesos
Description
-------
Updated operator API documention to use rereservation format.
Diffs (updated)
-----
docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
Diff: https://reviews.apache.org/r/71741/diff/2/
Changes: https://reviews.apache.org/r/71741/diff/1-2/
Testing
-------
Previewed in generated site
Thanks,
Benjamin Bannier
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218580
-----------------------------------------------------------
Bad review!
Reviews applied: [71741, 71739, 71725, 71729]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On Nov. 8, 2019, 1:13 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2019, 1:13 p.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/1/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Nov. 18, 2019, 5:41 p.m., Benno Evers wrote:
> > I'm not sure that this example does much to explain the semantics of the `RESERVE_RESOURCES` call to unsuspecting readers: The example you modify will behave exactly the same whether the `source` field is present or not.
> >
> > At the very least, `source` should be formally introduced in the description of the `RESERVE_RESOURCES` call at the beginning of the section. Ideally, we'd have a first example without `source` (i.e. the one we had before) and then a second example using `source` to do an actual reservation update. (In your commit message you use the word `rereservation`, do we use that anywhere else?)
I updated the documentation and examples. I went with two examples (covering both "simple reservation" and re-reservation). I did however in both cases specify `source` as IMO it is the cleaner API.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218664
-----------------------------------------------------------
On Nov. 8, 2019, 2:13 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2019, 2:13 p.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/1/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71741: Updated operator API documention to use
rereservation format.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71741/#review218664
-----------------------------------------------------------
I'm not sure that this example does much to explain the semantics of the `RESERVE_RESOURCES` call to unsuspecting readers: The example you modify will behave exactly the same whether the `source` field is present or not.
At the very least, `source` should be formally introduced in the description of the `RESERVE_RESOURCES` call at the beginning of the section. Ideally, we'd have a first example without `source` (i.e. the one we had before) and then a second example using `source` to do an actual reservation update. (In your commit message you use the word `rereservation`, do we use that anywhere else?)
- Benno Evers
On Nov. 8, 2019, 1:13 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71741/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2019, 1:13 p.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9993
> https://issues.apache.org/jira/browse/MESOS-9993
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated operator API documention to use rereservation format.
>
>
> Diffs
> -----
>
> docs/operator-http-api.md 6cc367dabf3bec507e8d49af54d9aad9ac17471c
>
>
> Diff: https://reviews.apache.org/r/71741/diff/1/
>
>
> Testing
> -------
>
> Previewed in generated site
>
>
> Thanks,
>
> Benjamin Bannier
>
>