You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexander Rukletsov <al...@mesosphere.io> on 2015/03/05 22:04:51 UTC
Review Request 31776: Moved allocator to public headers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 580e1f818201f951c11e4e652a7941fcd888356d
src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On March 17, 2015, 4:58 p.m., Kapil Arya wrote:
> > src/master/allocator/allocator.hpp, line 39
> > <https://reviews.apache.org/r/31776/diff/3/?file=894463#file894463line39>
> >
> > Why do we have a separate allocator namespace? Wouldn't it be better to just put the Allocator class in `mesos::master`?
>
> Alexander Rukletsov wrote:
> Namespace `allocator` contains everything related to resource allocation, i.e. also `Sorter`s, `Comparator`s. Since sorters are poorly named, I do not want to pollute `master` namespace. Let's create a JIRA for now: https://issues.apache.org/jira/browse/MESOS-2516.
That works. Please drop the issue.
> On March 17, 2015, 4:58 p.m., Kapil Arya wrote:
> > src/master/allocator/mesos/allocator.hpp, line 32
> > <https://reviews.apache.org/r/31776/diff/3/?file=894464#file894464line32>
> >
> > Same as above. Why not kill the allocator namespace? Afterall, everything in this file has an "Allocator" suffix.
>
> Alexander Rukletsov wrote:
> See above.
Please drop the issue.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review76805
-----------------------------------------------------------
On March 23, 2015, 10:02 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 10:02 a.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 17, 2015, 8:58 p.m., Kapil Arya wrote:
> > src/master/allocator/allocator.hpp, line 39
> > <https://reviews.apache.org/r/31776/diff/3/?file=894463#file894463line39>
> >
> > Why do we have a separate allocator namespace? Wouldn't it be better to just put the Allocator class in `mesos::master`?
Namespace `allocator` contains everything related to resource allocation, i.e. also `Sorter`s, `Comparator`s. Since sorters are poorly named, I do not want to pollute `master` namespace. Let's create a JIRA for now: https://issues.apache.org/jira/browse/MESOS-2516.
> On March 17, 2015, 8:58 p.m., Kapil Arya wrote:
> > src/master/allocator/mesos/allocator.hpp, line 32
> > <https://reviews.apache.org/r/31776/diff/3/?file=894464#file894464line32>
> >
> > Same as above. Why not kill the allocator namespace? Afterall, everything in this file has an "Allocator" suffix.
See above.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review76805
-----------------------------------------------------------
On March 13, 2015, 9:42 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 13, 2015, 9:42 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review76805
-----------------------------------------------------------
Ship it!
The review looks good to me. However I have added one issue related to the namespace. I believe this review doesn't need to address them, so may be you can create a Jira or something.
src/master/allocator/allocator.hpp
<https://reviews.apache.org/r/31776/#comment124463>
Why do we have a separate allocator namespace? Wouldn't it be better to just put the Allocator class in `mesos::master`?
src/master/allocator/mesos/allocator.hpp
<https://reviews.apache.org/r/31776/#comment124464>
Same as above. Why not kill the allocator namespace? Afterall, everything in this file has an "Allocator" suffix.
- Kapil Arya
On March 13, 2015, 5:42 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 13, 2015, 5:42 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 2, 2015, 6:23 p.m., Vinod Kone wrote:
> > include/mesos/master/allocator.proto, lines 19-23
> > <https://reviews.apache.org/r/31776/diff/6/?file=912077#file912077line19>
> >
> > Since allocator is within the same Unix process as Master, what is the compatibility issue here?
>
> Alexander Rukletsov wrote:
> The comment is related to the protobuf namespace in which `RoleInfo` lives. Not sure I'm getting what you mean by mentioning that allocator and Master are in the same Unix process.
I'll create a separate review request for this.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review78690
-----------------------------------------------------------
On April 15, 2015, 2:16 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:16 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am d15a37365bcdd5c3906160b46b389635b38b1673
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3
> src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36
> src/messages/messages.proto 2d242dcfc54f9146721a1b8fbef02e4de050cf58
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
> src/tests/mesos.cpp fc534e9febed1e293076e00e0f5c3879a78df90f
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 2, 2015, 6:23 p.m., Vinod Kone wrote:
> > include/mesos/master/allocator.proto, lines 19-23
> > <https://reviews.apache.org/r/31776/diff/6/?file=912077#file912077line19>
> >
> > Since allocator is within the same Unix process as Master, what is the compatibility issue here?
The comment is related to the protobuf namespace in which `RoleInfo` lives. Not sure I'm getting what you mean by mentioning that allocator and Master are in the same Unix process.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review78690
-----------------------------------------------------------
On April 1, 2015, 2:25 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 2:25 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 05be911734b8d70c9fa5f3b4a275e8b610621fc5
> src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp 2bfd0f8a20169649cff03509556b4cfa965a9837
> src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751
> src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 2, 2015, 6:23 p.m., Vinod Kone wrote:
> > src/Makefile.am, line 711
> > <https://reviews.apache.org/r/31776/diff/6/?file=912078#file912078line711>
> >
> > not yours, but can you kill the trailing white space here?
Sure.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review78690
-----------------------------------------------------------
On April 1, 2015, 2:25 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 2:25 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 05be911734b8d70c9fa5f3b4a275e8b610621fc5
> src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp 2bfd0f8a20169649cff03509556b4cfa965a9837
> src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751
> src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review78690
-----------------------------------------------------------
Ship it!
include/mesos/master/allocator.proto
<https://reviews.apache.org/r/31776/#comment127643>
Since allocator is within the same Unix process as Master, what is the compatibility issue here?
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment127644>
not yours, but can you kill the trailing white space here?
- Vinod Kone
On April 1, 2015, 2:25 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 2:25 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 05be911734b8d70c9fa5f3b4a275e8b610621fc5
> src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp 2bfd0f8a20169649cff03509556b4cfa965a9837
> src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751
> src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated April 20, 2015, 1:36 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Addressed Vinod's comment.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am d15a37365bcdd5c3906160b46b389635b38b1673
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/allocator.hpp 5e3e6138a36ddd6852ae1875cb967ad8487644e8
src/master/allocator/mesos/allocator.hpp af27a9bd8299cbff01e04b74db47c86bf247b908
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp c10e7c08c191acef9d31d98018a47a2a952a4dfc
src/master/master.cpp e30b951eda2b3b0d5b2a80716f0b32c6bbe041bc
src/messages/messages.proto 2d242dcfc54f9146721a1b8fbef02e4de050cf58
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
src/tests/hierarchical_allocator_tests.cpp 0b564a74d3f04df46fe52fcbe1bf8d4d1e41c53c
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666
src/tests/mesos.hpp 7744df55a2a31446327da7bd2b16457e90711d22
src/tests/mesos.cpp fc534e9febed1e293076e00e0f5c3879a78df90f
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, CentOS 7.0)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated April 15, 2015, 2:16 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Rebased.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am d15a37365bcdd5c3906160b46b389635b38b1673
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3
src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36
src/messages/messages.proto 2d242dcfc54f9146721a1b8fbef02e4de050cf58
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666
src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
src/tests/mesos.cpp fc534e9febed1e293076e00e0f5c3879a78df90f
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, CentOS 7.0)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated April 7, 2015, 12:46 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Fixed a space.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 05be911734b8d70c9fa5f3b4a275e8b610621fc5
src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 2bfd0f8a20169649cff03509556b4cfa965a9837
src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751
src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, CentOS 7.0)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated April 1, 2015, 2:25 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Rebased.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 05be911734b8d70c9fa5f3b4a275e8b610621fc5
src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 2bfd0f8a20169649cff03509556b4cfa965a9837
src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751
src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, CentOS 7.0)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated March 27, 2015, 4:25 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Addressed comments.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
src/master/master.cpp ffccc1259b62db89404f6aa6225beeb4c9828684
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf
src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
src/tests/master_authorization_tests.cpp ac79303645cc1af337b1dca8db244113d0ba6fce
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 2bfd0f8a20169649cff03509556b4cfa965a9837
src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751
src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, CentOS 7.0)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Michael Park <mc...@gmail.com>.
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > src/Makefile.am, lines 234-239
> > <https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line234>
> >
> > In line with the above comment, it seems like `$(ALLOCATOR_PROTO)` should live in `include/mesos/allocator/...` rather than `include/mesos/master/...`. Similar to `authentication`, `containerizer`, and `fetcher`.
>
> Kapil Arya wrote:
> The allocator proto should live in include/mesos/master since it's specific to mesos master. The containerizer/fetcher protos are for framework writers and are in their own subdirs for historic reasons. The authentication stuff used by both master and slave and hence in its own subdir. The correct precedence for this would be the stuff in include/mesos/slave.
Interesting, thanks @karya!
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
On March 23, 2015, 2:02 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2:02 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On March 23, 2015, 6:27 p.m., Michael Park wrote:
> > src/Makefile.am, lines 234-239
> > <https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line234>
> >
> > In line with the above comment, it seems like `$(ALLOCATOR_PROTO)` should live in `include/mesos/allocator/...` rather than `include/mesos/master/...`. Similar to `authentication`, `containerizer`, and `fetcher`.
The allocator proto should live in include/mesos/master since it's specific to mesos master. The containerizer/fetcher protos are for framework writers and are in their own subdirs for historic reasons. The authentication stuff used by both master and slave and hence in its own subdir. The correct precedence for this would be the stuff in include/mesos/slave.
> On March 23, 2015, 6:27 p.m., Michael Park wrote:
> > src/Makefile.am, lines 170-171
> > <https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line170>
> >
> > Seems to me like these should be `allocator/allocator.pb.cc`, and `../include/mesos/allocator/allocator.pb.h`. Could you explain why these are under `master`?
See below.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
On March 23, 2015, 10:02 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 10:02 a.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Michael Park <mc...@gmail.com>.
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > include/mesos/master/allocator.proto, lines 20-21
> > <https://reviews.apache.org/r/31776/diff/4/?file=902995#file902995line20>
> >
> > `s/breaking the backward compatibility/breaking backwards compatibility/`
> >
> > Also this is referring to the fact that we can't communicate with the old master if we pull this message out of `internal`, correct?
>
> Alexander Rukletsov wrote:
> Correct, namespace is part of the type.
Cool, thanks.
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > src/local/local.hpp, line 28
> > <https://reviews.apache.org/r/31776/diff/4/?file=902997#file902997line28>
> >
> > This comment looks useless. Can we just get rid of it?
>
> Alexander Rojas wrote:
> +1
>
> Alexander Rukletsov wrote:
> Totally agree. However, we have a lot of them in our codebase. I would like to get rid of them, but let's do it in a consistent manner: https://issues.apache.org/jira/browse/MESOS-2564
Sounds good to me. Still, staying consistent would look like this:
```
namespace mesos {
namespace internal {
// Forward declarations.
namespace master {
class Master;
} // namespace master {
class Configuration;
namespace local {
// Launch a local cluster with the given flags.
process::PID<master::Master> launch(
const Flags& flags,
mesos::master::allocator::Allocator* _allocator = NULL);
void shutdown();
} // namespace local {
} // namespace internal
} // namespace mesos
```
`Master` and `Configuration` are the forward declarations.
Followed by the declarations being made in the `local` namespace.
The `// Forward declarations.` comment being on top of the `mesos` namespace would indicate that the whole file is forward declarations.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
On March 23, 2015, 2:02 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2:02 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > include/mesos/master/allocator.proto, lines 20-21
> > <https://reviews.apache.org/r/31776/diff/4/?file=902995#file902995line20>
> >
> > `s/breaking the backward compatibility/breaking backwards compatibility/`
> >
> > Also this is referring to the fact that we can't communicate with the old master if we pull this message out of `internal`, correct?
Correct, namespace is part of the type.
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > src/Makefile.am, line 709
> > <https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line709>
> >
> > Is this a trailing whitespace? Not exactly sure what red highlight means.
No, rather RB's quirk : )
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > src/local/local.hpp, line 28
> > <https://reviews.apache.org/r/31776/diff/4/?file=902997#file902997line28>
> >
> > This comment looks useless. Can we just get rid of it?
>
> Alexander Rojas wrote:
> +1
Totally agree. However, we have a lot of them in our codebase. I would like to get rid of them, but let's do it in a consistent manner: https://issues.apache.org/jira/browse/MESOS-2564
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
On March 23, 2015, 2:02 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2:02 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Michael Park <mc...@gmail.com>.
> On March 23, 2015, 10:27 p.m., Michael Park wrote:
> > src/Makefile.am, lines 170-171
> > <https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line170>
> >
> > Seems to me like these should be `allocator/allocator.pb.cc`, and `../include/mesos/allocator/allocator.pb.h`. Could you explain why these are under `master`?
>
> Kapil Arya wrote:
> See below.
Thanks, dropping issue.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
On March 23, 2015, 2:02 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2:02 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On March 23, 2015, 11:27 p.m., Michael Park wrote:
> > src/local/local.hpp, line 28
> > <https://reviews.apache.org/r/31776/diff/4/?file=902997#file902997line28>
> >
> > This comment looks useless. Can we just get rid of it?
+1
> On March 23, 2015, 11:27 p.m., Michael Park wrote:
> > src/master/main.cpp, lines 204-205
> > <https://reviews.apache.org/r/31776/diff/4/?file=903001#file903001line204>
> >
> > Indent 2 spaces for assignment.
+1
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
On March 23, 2015, 3:02 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 3:02 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review77477
-----------------------------------------------------------
include/mesos/master/allocator.proto
<https://reviews.apache.org/r/31776/#comment125569>
`s/breaking the backward compatibility/breaking backwards compatibility/`
Also this is referring to the fact that we can't communicate with the old master if we pull this message out of `internal`, correct?
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment125565>
Seems to me like these should be `allocator/allocator.pb.cc`, and `../include/mesos/allocator/allocator.pb.h`. Could you explain why these are under `master`?
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment125564>
In line with the above comment, it seems like `$(ALLOCATOR_PROTO)` should live in `include/mesos/allocator/...` rather than `include/mesos/master/...`. Similar to `authentication`, `containerizer`, and `fetcher`.
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment125563>
Is this a trailing whitespace? Not exactly sure what red highlight means.
src/local/local.hpp
<https://reviews.apache.org/r/31776/#comment125570>
This comment looks useless. Can we just get rid of it?
src/master/main.cpp
<https://reviews.apache.org/r/31776/#comment125571>
Indent 2 spaces for assignment.
- Michael Park
On March 23, 2015, 2:02 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2:02 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, CentOS 7.0)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated March 23, 2015, 2:02 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Rebased.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing (updated)
-------
make check (Mac OS 10.9.5, CentOS 7.0)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated March 13, 2015, 9:42 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Addressed Niklas' comments.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7
src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 10, 2015, 9:09 p.m., Niklas Nielsen wrote:
> > src/local/local.hpp, lines 26-46
> > <https://reviews.apache.org/r/31776/diff/2/?file=886202#file886202line26>
> >
> > Can we include the allocator and master header instead?
Let's include allocator header and leave fwd decl master to keep build times shorter. The former is a public header => should not change that often, the latter is an internal one and may change frequently.
> On March 10, 2015, 9:09 p.m., Niklas Nielsen wrote:
> > src/local/local.hpp, line 29
> > <https://reviews.apache.org/r/31776/diff/2/?file=886202#file886202line29>
> >
> > Let's create a JIRA ticket for the namespace rules and get it in the style guide. I initially found it hard to read with the new line here, but after a conversation offline, I found that it's a rule we (mostly) apply: group namespace beginnings when they are closed together:
> >
> > namespace A {
> >
> > namespace B {
> > namespace C {
> > } // namespace B {
> > } // namespace C {
> >
> > namespace D {
> > } // namespace D {
> > } // namespace A {
https://issues.apache.org/jira/browse/MESOS-2488
> On March 10, 2015, 9:09 p.m., Niklas Nielsen wrote:
> > src/master/master.hpp, lines 75-80
> > <https://reviews.apache.org/r/31776/diff/2/?file=886207#file886207line75>
> >
> > Why do you need this forward declaration when you have the allocator include?
Good catch!
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review75894
-----------------------------------------------------------
On March 5, 2015, 9:11 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 5, 2015, 9:11 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp 580e1f818201f951c11e4e652a7941fcd888356d
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/#review75894
-----------------------------------------------------------
include/mesos/master/allocator.proto
<https://reviews.apache.org/r/31776/#comment123311>
Can you add a comment on why this is in internal
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment123221>
Let's keep this sorted
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment123222>
Let's keep this ordering in sync with the list above
src/Makefile.am
<https://reviews.apache.org/r/31776/#comment123223>
Again, let's keep ordering :)
src/local/local.hpp
<https://reviews.apache.org/r/31776/#comment123227>
Can we include the allocator and master header instead?
src/local/local.hpp
<https://reviews.apache.org/r/31776/#comment123226>
Let's create a JIRA ticket for the namespace rules and get it in the style guide. I initially found it hard to read with the new line here, but after a conversation offline, I found that it's a rule we (mostly) apply: group namespace beginnings when they are closed together:
namespace A {
namespace B {
namespace C {
} // namespace B {
} // namespace C {
namespace D {
} // namespace D {
} // namespace A {
src/master/allocator/allocator.hpp
<https://reviews.apache.org/r/31776/#comment123312>
Let's move the comment above and insert a newline below
src/master/master.hpp
<https://reviews.apache.org/r/31776/#comment123228>
Why do you need this forward declaration when you have the allocator include?
- Niklas Nielsen
On March 5, 2015, 1:11 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31776/
> -----------------------------------------------------------
>
> (Updated March 5, 2015, 1:11 p.m.)
>
>
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
>
>
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.proto PRE-CREATION
> src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f
> src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
> src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
> src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec
> src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
> src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
> src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
> src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
> src/tests/master_tests.cpp 580e1f818201f951c11e4e652a7941fcd888356d
> src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
> src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
> src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
>
> Diff: https://reviews.apache.org/r/31776/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31776: Moved allocator to public headers.
Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31776/
-----------------------------------------------------------
(Updated March 5, 2015, 9:11 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Fixed style issue.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
This is required for out-of-tree allocator modules. RoleInfo protobuf message has to be extracted into its own public proto file.
Diffs (updated)
-----
include/mesos/master/allocator.proto PRE-CREATION
src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f
src/local/local.hpp 0aa50ef4f44c347eca4b3a409f2a8d03ba321d82
src/local/local.cpp 19083368212b24ce1afef3a5f91d48766d1cd55e
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d
src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec
src/messages/messages.proto 97c45c01dfcea38b1ae555c036d61e10c152c2c8
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/fault_tolerance_tests.cpp 9ac75b1f601e14a3d3d117775f37a4a48b291dc6
src/tests/hierarchical_allocator_tests.cpp 93753d1c04159a04a733927a487eb69505438e32
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
src/tests/master_authorization_tests.cpp ff706ed6f8537207b30a548b0ce2121c5df71ab9
src/tests/master_slave_reconciliation_tests.cpp e60f601202fcdbb4cafac190e9b09ca6ce925260
src/tests/master_tests.cpp 580e1f818201f951c11e4e652a7941fcd888356d
src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8
src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1
src/tests/rate_limiting_tests.cpp d5c00b8db30fc529bc984417a632cf99eb50eb28
Diff: https://reviews.apache.org/r/31776/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov