You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@cn.ibm.com> on 2016/01/17 17:32:34 UTC

Review Request 42415: Added a roles field to FrameworkInfo.

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

Review request for mesos, Benjamin Bannier and Vinod Kone.


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


Repository: mesos


Description
-------

Added a roles field to FrameworkInfo.


Diffs
-----

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 

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


Testing
-------

make check (Mac OS X 10.10.5)


Thanks,

Qian Zhang


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Qian Zhang <zh...@cn.ibm.com>.

> On Jan. 18, 2016, 6:34 p.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto, line 241
> > <https://reviews.apache.org/r/42415/diff/1/?file=1199455#file1199455line241>
> >
> >     This is pretty useless as it does not allow multiple roles. We do need a field `Repeated string roles` here like is written explicitly in the design doc.

My bad, sorry about that.


- Qian


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


On Jan. 19, 2016, 8:25 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 8:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/#review114980
-----------------------------------------------------------



include/mesos/mesos.proto (line 241)
<https://reviews.apache.org/r/42415/#comment175823>

    This is pretty useless as it does not allow multiple roles. We do need a field `Repeated string roles` here like is written explicitly in the design doc.



include/mesos/v1/mesos.proto (line 241)
<https://reviews.apache.org/r/42415/#comment175824>

    This is pretty useless as it does not allow multiple roles. We do need a field `Repeated string roles` here like is written explicitly in the design doc.


- Benjamin Bannier


On Jan. 17, 2016, 5:32 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2016, 5:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/#review114911
-----------------------------------------------------------


Patch looks great!

Reviews applied: [42415]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 17, 2016, 4:32 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2016, 4:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Qian Zhang <zh...@cn.ibm.com>.

> On Jan. 19, 2016, 5:08 p.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto, lines 238-240
> > <https://reviews.apache.org/r/42415/diff/2/?file=1200833#file1200833line238>
> >
> >     Let's break this down into multiple pieces. How about
> >     
> >         // A framework can have multiple roles. Either one of `role` or `roles`
> >         // can be set, but never both. The `role` field will be removed once its
> >         // deprecation cycle is over.

Agree.


> On Jan. 19, 2016, 5:08 p.m., Benjamin Bannier wrote:
> > include/mesos/v1/mesos.proto, lines 238-240
> > <https://reviews.apache.org/r/42415/diff/2/?file=1200834#file1200834line238>
> >
> >     Same here.

Agree.


- Qian


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


On Jan. 19, 2016, 8:25 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 8:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/#review115134
-----------------------------------------------------------



include/mesos/mesos.proto (lines 238 - 240)
<https://reviews.apache.org/r/42415/#comment175994>

    Let's break this down into multiple pieces. How about
    
        // A framework can have multiple roles. Either one of `role` or `roles`
        // can be set, but never both. The `role` field will be removed once its
        // deprecation cycle is over.



include/mesos/v1/mesos.proto (lines 238 - 240)
<https://reviews.apache.org/r/42415/#comment175995>

    Same here.


- Benjamin Bannier


On Jan. 19, 2016, 1:25 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/#review115103
-----------------------------------------------------------


Patch looks great!

Reviews applied: [42415]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 19, 2016, 12:25 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/#review115399
-----------------------------------------------------------

Ship it!


- Benjamin Bannier


On Jan. 19, 2016, 1:45 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/#review115439
-----------------------------------------------------------


Patch looks great!

Reviews applied: [42415]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 19, 2016, 12:45 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 12:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
>     https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/
-----------------------------------------------------------

(Updated Jan. 19, 2016, 8:45 p.m.)


Review request for mesos, Benjamin Bannier and Vinod Kone.


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


Repository: mesos


Description
-------

Added a roles field to FrameworkInfo.


Diffs (updated)
-----

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 

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


Testing
-------

make check (Mac OS X 10.10.5)


Thanks,

Qian Zhang


Re: Review Request 42415: Added a roles field to FrameworkInfo.

Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42415/
-----------------------------------------------------------

(Updated Jan. 19, 2016, 8:25 a.m.)


Review request for mesos, Benjamin Bannier and Vinod Kone.


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


Repository: mesos


Description
-------

Added a roles field to FrameworkInfo.


Diffs (updated)
-----

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 

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


Testing
-------

make check (Mac OS X 10.10.5)


Thanks,

Qian Zhang