Re: Review Request 62877: Added RESOURCE_PROVIDER agent capability.

[Greg Mann]

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



I noticed that we have an agent capabilities test here: 
https://github.com/apache/mesos/blob/d5630f3412dab3b3464f59c57b7526068ebbcb96/src/tests/protobuf_utils_tests.cpp#L278-L292

I'm not convinced that test is necessary, given how simple the constructor is 
and the fact that other tests exercise the MULTI_ROLE capability and would fail 
if the constructor doesn't work.
I just wanted to give you a heads up on that test in case you thought it was 
beneficial and wanted to add your new capability to it.

- Greg Mann


On Oct. 17, 2017, 11:24 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62877/
> ---
> 
> (Updated Oct. 17, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8071
> https://issues.apache.org/jira/browse/MESOS-8071
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added RESOURCE_PROVIDER agent capability.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
> 
> 
> Diff: https://reviews.apache.org/r/62877/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62877: Added RESOURCE_PROVIDER agent capability.

[Greg Mann]

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


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 972-974 (patched)


Will this fit on two lines? Here and below.



include/mesos/mesos.proto
Lines 976 (patched)


s/provider/provide/

Here and below.


- Greg Mann


On Oct. 17, 2017, 11:24 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62877/
> ---
> 
> (Updated Oct. 17, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8071
> https://issues.apache.org/jira/browse/MESOS-8071
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added RESOURCE_PROVIDER agent capability.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
> 
> 
> Diff: https://reviews.apache.org/r/62877/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62877: Added RESOURCE_PROVIDER agent capability.

[Jie Yu]

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

(Updated Oct. 17, 2017, 11:24 p.m.)


Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added RESOURCE_PROVIDER agent capability.


Diffs (updated)
-

  include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32 
  include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
  src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
  src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 


Diff: https://reviews.apache.org/r/62877/diff/3/

Changes: https://reviews.apache.org/r/62877/diff/2-3/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 62877: Added RESOURCE_PROVIDER agent capability.

[Jie Yu]

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

(Updated Oct. 17, 2017, 6:59 p.m.)


Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added RESOURCE_PROVIDER agent capability.


Diffs (updated)
-

  include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32 
  include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
  src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
  src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 


Diff: https://reviews.apache.org/r/62877/diff/2/

Changes: https://reviews.apache.org/r/62877/diff/1-2/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 62877: Added RESOURCE_PROVIDER agent capability.

[Jie Yu]

> On Oct. 12, 2017, 3:04 p.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto
> > Lines 958-966 (patched)
> > 
> >
> > I do not feel very strongly about this, but it still feels to me that 
> > this capability is less about resource providers and more about the agent 
> > being able to provide a protocol for offer operation feedback.
> > 
> > The capability does also show up in e.g., the `/master/slaves` 
> > endpoint. I could imagine frameworks being interested in looking at this 
> > capability to follow different logic for offer operation feedback. Still, 
> > resource providers would not be directly visible to them.
> > 
> > From an implementation point of view, we will add code to the master 
> > providing differing protocols for offer operations branching on the 
> > presence of this capability. We again mostly would not care about resource 
> > providers in that case either.

I agree it's a combination of multiple things, but I think this is OK


- Jie


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


On Oct. 11, 2017, 3:24 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62877/
> ---
> 
> (Updated Oct. 11, 2017, 3:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8071
> https://issues.apache.org/jira/browse/MESOS-8071
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added RESOURCE_PROVIDER agent capability.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 830985a3265b7c104d8fdc50749c395d98f5f3c8 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
> 
> 
> Diff: https://reviews.apache.org/r/62877/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62877: Added RESOURCE_PROVIDER agent capability.

[Benjamin Bannier]

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


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 958-966 (patched)


I do not feel very strongly about this, but it still feels to me that this 
capability is less about resource providers and more about the agent being able 
to provide a protocol for offer operation feedback.

The capability does also show up in e.g., the `/master/slaves` endpoint. I 
could imagine frameworks being interested in looking at this capability to 
follow different logic for offer operation feedback. Still, resource providers 
would not be directly visible to them.

From an implementation point of view, we will add code to the master 
providing differing protocols for offer operations branching on the presence of 
this capability. We again mostly would not care about resource providers in 
that case either.


- Benjamin Bannier


On Oct. 11, 2017, 5:24 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62877/
> ---
> 
> (Updated Oct. 11, 2017, 5:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8071
> https://issues.apache.org/jira/browse/MESOS-8071
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added RESOURCE_PROVIDER agent capability.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 830985a3265b7c104d8fdc50749c395d98f5f3c8 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
> 
> 
> Diff: https://reviews.apache.org/r/62877/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>