Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-04-08 Thread Jie Yu 

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

Ship it!


Ship It!

- Jie Yu


On April 7, 2015, 9:52 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated April 7, 2015, 9:52 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: Add 'Resource::ReservationInfo' protobuf message.

2015-04-07 Thread Michael Park 

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

(Updated April 7, 2015, 9:51 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 
`framework_id` field. We don't need the required `principal` field yet and will 
be introduced together with the "unreserve" ACLs. An optional `id` may be 
introduced in the future as well.


Diffs (updated)
-

  include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 

Diff: https://reviews.apache.org/r/32139/diff/


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-04-07 Thread Michael Park 

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

(Updated April 7, 2015, 9:52 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 (updated)
---

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: Add 'Resource::ReservationInfo' protobuf message.

2015-03-25 Thread Alexander Rukletsov 

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



include/mesos/mesos.proto


MPark, sorry if I leave the comment in a wrong place, I don't know where we 
discuss the design now. I'm sure you'll point me to the right doc : ).

Q1: Do we allow frameworks to reserve dynamically for role, or just 
operators?

Q2: If the answer to Q1 is "no", does it make sense to distinguish between 
static reservations and dynamic reservations since both of them are done by an 
operator?

Q3: If the answer to Q2 is "yes", how are we going to distinguish between 
framework and operator dynamic *role* reservations if we introduce framework 
role reservations one day?



include/mesos/mesos.proto


Let's insert a whitespace `"*" | R` if you are going to edit this.


- Alexander Rukletsov


On March 25, 2015, 6:47 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated March 25, 2015, 6:47 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 
> `framework_id` field. We don't need the required `principal` field yet and 
> will be introduced together with the "unreserve" ACLs. An optional `id` may 
> be introduced in the future as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
> 
> Diff: https://reviews.apache.org/r/32139/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-25 Thread Michael Park 

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

(Updated March 25, 2015, 6:47 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.


Changes
---

Updated the comment for the `reservation` field, Removed the list of valid 
states/transitions for (role, reservation) pair. I'll update this with a link 
to the user guide when the user guide is ready to be added to the repo.


Bugs: MESOS-2475
https://issues.apache.org/jira/browse/MESOS-2475


Repository: mesos


Description
---

The beginning of `Resource::ReservationInfo`. This patch only introduces the 
`framework_id` field. We don't need the required `principal` field yet and will 
be introduced together with the "unreserve" ACLs. An optional `id` may be 
introduced in the future as well.


Diffs (updated)
-

  include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 

Diff: https://reviews.apache.org/r/32139/diff/


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-25 Thread Michael Park 


> On March 24, 2015, 7:25 a.m., Timothy Chen wrote:
> > include/mesos/mesos.proto, line 397
> > 
> >
> > Not sure this is the right place to comment on the valid specifications 
> > of both role and reservation. There is a user guide added right? We should 
> > refer to that instead.
> 
> Michael Park wrote:
> Hm, the user guide doesn't exist in the repo. Maybe we just say something 
> like: "Refer to the reservations user guide for the valid states and 
> transitions of (role, reservation) pair." and add in the links together with 
> the user guide later?

It seems weird to refer to a user guide that doesn't exist yet. I'll add the 
comment here with the link to the user guide when the user guide is ready to be 
added to the repo.


- Michael


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


On March 25, 2015, 6:47 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated March 25, 2015, 6:47 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 
> `framework_id` field. We don't need the required `principal` field yet and 
> will be introduced together with the "unreserve" ACLs. An optional `id` may 
> be introduced in the future as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
> 
> Diff: https://reviews.apache.org/r/32139/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-25 Thread Michael Park 


> On March 24, 2015, 7:25 a.m., Timothy Chen wrote:
> > include/mesos/mesos.proto, line 397
> > 
> >
> > Not sure this is the right place to comment on the valid specifications 
> > of both role and reservation. There is a user guide added right? We should 
> > refer to that instead.

Hm, the user guide doesn't exist in the repo. Maybe we just say something like: 
"Refer to the reservations user guide for the valid states and transitions of 
(role, reservation) pair." and add in the links together with the user guide 
later?


> On March 24, 2015, 7:25 a.m., Timothy Chen wrote:
> > include/mesos/mesos.proto, line 428
> > 
> >
> > It also can be unreserved as well?

If the `reservation` is not set, it's either "statically reserved" or 
"unreserved". I cheated here by saying "statically configured", perhaps that's 
confusing. I'll explicitly mention "statically reserved" and "unreserved".


- Michael


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


On March 18, 2015, 7:43 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated March 18, 2015, 7:43 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 
> `framework_id` field. We don't need the required `principal` field yet and 
> will be introduced together with the "unreserve" ACLs. An optional `id` may 
> be introduced in the future as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ec8efaec13f54a56d82411f6cdbdb8ad8b103748 
> 
> Diff: https://reviews.apache.org/r/32139/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-24 Thread Timothy Chen 

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



include/mesos/mesos.proto


Not sure this is the right place to comment on the valid specifications of 
both role and reservation. There is a user guide added right? We should refer 
to that instead.



include/mesos/mesos.proto


It also can be unreserved as well?


- Timothy Chen


On March 18, 2015, 7:43 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated March 18, 2015, 7:43 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 
> `framework_id` field. We don't need the required `principal` field yet and 
> will be introduced together with the "unreserve" ACLs. An optional `id` may 
> be introduced in the future as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ec8efaec13f54a56d82411f6cdbdb8ad8b103748 
> 
> Diff: https://reviews.apache.org/r/32139/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-18 Thread Michael Park 


> On March 18, 2015, 12:27 a.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, lines 412-415
> > 
> >
> > Are you saying "Resources cannot be reserved for '*' role"? Or there is 
> > more information that it's hard for me to understand?
> > 
> > Q about design: why a framework from role `'*'` cannot reserve 
> > resources?

My mistake. Frameworks with role "*" can reserve resources. I've fixed this 
comment and added a section regarding valid states and transitions of the 
(role, reservation) pair.


- Michael


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


On March 18, 2015, 7:43 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated March 18, 2015, 7:43 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 
> `framework_id` field. We don't need the required `principal` field yet and 
> will be introduced together with the "unreserve" ACLs. An optional `id` may 
> be introduced in the future as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ec8efaec13f54a56d82411f6cdbdb8ad8b103748 
> 
> Diff: https://reviews.apache.org/r/32139/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-18 Thread Michael Park 

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

(Updated March 18, 2015, 7:43 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.


Changes
---

Addressed Alex's comments.


Bugs: MESOS-2475
https://issues.apache.org/jira/browse/MESOS-2475


Repository: mesos


Description
---

The beginning of `Resource::ReservationInfo`. This patch only introduces the 
`framework_id` field. We don't need the required `principal` field yet and will 
be introduced together with the "unreserve" ACLs. An optional `id` may be 
introduced in the future as well.


Diffs (updated)
-

  include/mesos/mesos.proto ec8efaec13f54a56d82411f6cdbdb8ad8b103748 

Diff: https://reviews.apache.org/r/32139/diff/


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-17 Thread Alexander Rukletsov 

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



include/mesos/mesos.proto


Are you saying "Resources cannot be reserved for '*' role"? Or there is 
more information that it's hard for me to understand?

Q about design: why a framework from role `'*'` cannot reserve resources?



include/mesos/mesos.proto


s/static/statically ?


- Alexander Rukletsov


On March 17, 2015, 3:19 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32139/
> ---
> 
> (Updated March 17, 2015, 3:19 a.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 
> `framework_id` field. We don't need the required `principal` field yet and 
> will be introduced together with the "unreserve" ACLs. An optional `id` may 
> be introduced in the future as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 
> 
> Diff: https://reviews.apache.org/r/32139/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 32139: Add 'Resource::ReservationInfo' protobuf message.

2015-03-16 Thread Michael Park 

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

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 
`framework_id` field. We don't need the required `principal` field yet and will 
be introduced together with the "unreserve" ACLs. An optional `id` may be 
introduced in the future as well.


Diffs
-

  include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 

Diff: https://reviews.apache.org/r/32139/diff/


Testing
---

make check


Thanks,

Michael Park