> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > include/mesos/master/allocator.proto, line 19
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155772#file1155772line19>
> >
> >     Shouldn't this file have `java_package` and `java_outer_classname` just 
> > like the other protos?
> >     Looks like isolator.proto and oversubscription.proto are missing it 
> > too. Would you mind creating a separate patch to fix that?

I am not sure if we need to add java_package and java_outer_classname in those 
proto files, can you please clarify a little more about why we need to do this?


> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > include/mesos/role/role.proto, line 21
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155774#file1155774line21>
> >
> >     Why change the package? Couldn't this still be in `mesos.master`? Then 
> > you wouldn't have to change all the other files.

Like other feature(such as quota), I also think role manamgnet is a seprated 
function, so I define role protobuf in a separated package rather than define 
it in mesos.proto.


- Yongqiao


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


On Dec. 8, 2015, 5:20 a.m., Yongqiao Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 5:20 a.m.)
> 
> 
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
> 
> 
> Bugs: MESOS-3944
>     https://issues.apache.org/jira/browse/MESOS-3944
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently role protobuf is defined in allocator.proto due to only the 
> traditional DRF allocator uses roles as it’s first level of hierarchy, I 
> think we should move it out and define it in a separated file as quota had in 
> dynamic roles project, because role protobuf will also be used by master to 
> persist.
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   include/mesos/role/role.hpp PRE-CREATION 
>   include/mesos/role/role.proto PRE-CREATION 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
> 
> Diff: https://reviews.apache.org/r/40431/diff/
> 
> 
> Testing
> -------
> 
> 1. Make Check successfully;
> 
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Reply via email to