You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2015/04/24 10:19:09 UTC
Re: Review Request 32139: Added 'Resource::ReservationInfo' protobuf
message.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32139/#review81457
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/32139/#comment131838>
While reviewing `Resources::isDynamicReservation()` from https://reviews.apache.org/r/32398/ I realized that `ReservationInfo` describes _only_ dynamic reservation. I think this naming is a bit misleading. Do you have plans in mind to extend the scope of `ReservationInfo` beyond dynamic reservations and therefore keep the name general?
- Alexander Rukletsov
On April 21, 2015, 8:35 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> -----------------------------------------------------------
>
> (Updated April 21, 2015, 8:35 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.
>
>
> Bugs: MESOS-2475
> https://issues.apache.org/jira/browse/MESOS-2475
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The beginning of `Resource::ReservationInfo`. This patch only introduces the `principal` field.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537
>
> Diff: https://reviews.apache.org/r/32139/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 32139: Added 'Resource::ReservationInfo' protobuf
message.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 24, 2015, 8:19 a.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, lines 397-398
> > <https://reviews.apache.org/r/32139/diff/5/?file=938665#file938665line397>
> >
> > While reviewing `Resources::isDynamicReservation()` from https://reviews.apache.org/r/32398/ I realized that `ReservationInfo` describes _only_ dynamic reservation. I think this naming is a bit misleading. Do you have plans in mind to extend the scope of `ReservationInfo` beyond dynamic reservations and therefore keep the name general?
>
> Michael Park wrote:
> > Do you have plans in mind to extend the scope of ReservationInfo beyond dynamic reservations and therefore keep the name general?
>
> I think other types of reservations such as cluster-wide reservations could/would probably use this message to capture the information necessary. I'm inclined to keep it general for extensibility, but I can be convined otherwise.
What I find a bit misleading for new mesos users is that `ReservationInfo` doesn't include static reservations and doesn't hint in its naming that other kind of reservations exist. But you point makes sense to me and since you mention static reservations in the doc, I'm fine with keeping the current naming.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32139/#review81457
-----------------------------------------------------------
On April 27, 2015, 5:15 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> -----------------------------------------------------------
>
> (Updated April 27, 2015, 5:15 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.
>
>
> Bugs: MESOS-2475
> https://issues.apache.org/jira/browse/MESOS-2475
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The beginning of `Resource::ReservationInfo`. This patch only introduces the `principal` field.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 967b1e3bbfb3f6b71d5a15d02cba7ed5ec21816f
>
> Diff: https://reviews.apache.org/r/32139/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 32139: Added 'Resource::ReservationInfo' protobuf
message.
Posted by Michael Park <mc...@gmail.com>.
> On April 24, 2015, 8:19 a.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, lines 397-398
> > <https://reviews.apache.org/r/32139/diff/5/?file=938665#file938665line397>
> >
> > While reviewing `Resources::isDynamicReservation()` from https://reviews.apache.org/r/32398/ I realized that `ReservationInfo` describes _only_ dynamic reservation. I think this naming is a bit misleading. Do you have plans in mind to extend the scope of `ReservationInfo` beyond dynamic reservations and therefore keep the name general?
> Do you have plans in mind to extend the scope of ReservationInfo beyond dynamic reservations and therefore keep the name general?
I think other types of reservations such as cluster-wide reservations could/would probably use this message to capture the information necessary. I'm inclined to keep it general for extensibility, but I can be convined otherwise.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32139/#review81457
-----------------------------------------------------------
On April 27, 2015, 5:15 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> -----------------------------------------------------------
>
> (Updated April 27, 2015, 5:15 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.
>
>
> Bugs: MESOS-2475
> https://issues.apache.org/jira/browse/MESOS-2475
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The beginning of `Resource::ReservationInfo`. This patch only introduces the `principal` field.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 967b1e3bbfb3f6b71d5a15d02cba7ed5ec21816f
>
> Diff: https://reviews.apache.org/r/32139/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>