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:07:29 UTC
Re: Review Request 31265: Provided a factory for allocator in tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated March 5, 2015, 9:07 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
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
Diff: https://reviews.apache.org/r/31265/diff/
Testing (updated)
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 10, 2015, 9:11 p.m., Niklas Nielsen wrote:
> > How about we try to make this look like the Containerizer factory-like method (Containerizer::create)? That would be a bit more consistent
I think `Containerizer` is not a good example here. I would rather point to `Authenticator` and / or `Authenticatee`. We do not have `.create()` methods there, but we check in slave and master code whether we have modules for them. Therefore, I do not think we need a `.create()` function for `Allocator`.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review75978
-----------------------------------------------------------
On March 5, 2015, 9:07 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated March 5, 2015, 9:07 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review75978
-----------------------------------------------------------
How about we try to make this look like the Containerizer factory-like method (Containerizer::create)? That would be a bit more consistent
- Niklas Nielsen
On March 5, 2015, 1:07 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated March 5, 2015, 1:07 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Michael Park <mc...@gmail.com>.
> On March 24, 2015, 2:41 p.m., Michael Park wrote:
> > src/tests/master_allocator_tests.cpp, line 97
> > <https://reviews.apache.org/r/31265/diff/4/?file=903029#file903029line97>
> >
> > 1. Do we need this to be `virtual` right now?
> > 2. `s/createAllocatorInstance/CreateAllocator/`
> > 3. Can we move this either above `SetUp` or below `TearDown`? seems weird to have it sitting in the middle.
>
> Alexander Rukletsov wrote:
> 1. I though it make sense to mark it virtual, but I'll remove it if you think it's cleaner.
> 2. We use `camelCase`
> 3. Sure.
1. It wasn't about cleanliness, but rather that I don't think `createAllocator` is a function that should have polymorphic behavior.
2. Seems like we're inconsistent yet again. `src/tests/mesos.hpp` introduces a bunch of non-gtest functions such as `CreateMasterFlags` which are title-cased (I guess to keep consistent with gtest functions). `src/tests/persistent_volumes_tests.cpp` for example kept this pattern and has title-cased functions, but other tests such as `DockerContainerizerTest` in `src/tests/docker_containerizer_tests.cpp` did not.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review77570
-----------------------------------------------------------
On March 27, 2015, 4:27 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 4:27 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 24, 2015, 2:41 p.m., Michael Park wrote:
> > src/tests/master_allocator_tests.cpp, line 97
> > <https://reviews.apache.org/r/31265/diff/4/?file=903029#file903029line97>
> >
> > 1. Do we need this to be `virtual` right now?
> > 2. `s/createAllocatorInstance/CreateAllocator/`
> > 3. Can we move this either above `SetUp` or below `TearDown`? seems weird to have it sitting in the middle.
1. I though it make sense to mark it virtual, but I'll remove it if you think it's cleaner.
2. We use `camelCase`
3. Sure.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review77570
-----------------------------------------------------------
On March 23, 2015, 2:11 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2: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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review77570
-----------------------------------------------------------
src/tests/master_allocator_tests.cpp
<https://reviews.apache.org/r/31265/#comment125689>
1. Do we need this to be `virtual` right now?
2. `s/createAllocatorInstance/CreateAllocator/`
3. Can we move this either above `SetUp` or below `TearDown`? seems weird to have it sitting in the middle.
- Michael Park
On March 23, 2015, 2:11 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 2: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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Michael Park <mc...@gmail.com>.
> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 77
> > <https://reviews.apache.org/r/31265/diff/8/?file=929836#file929836line77>
> >
> > I see this is the place where you wanted to use `ASSERT_SOME` but couldn't due to the [gtest limitation](https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement).
> >
> > One way to solve this I think is to use `SetUp/TearDown` rather than `Ctor/Dtor`.
> > Another way may be to use `CHECK_SOME` in place of `ASSERT_SOME`. I think this makes sense as well since success of `HierarchicalDRFAllocator::create` isn't actually a testing condition.
> >
> > Thoughts?
Actually, is it possible just use a `TestAllocator` constructed from `TestAllocator::create<>()` as suggested below?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review80445
-----------------------------------------------------------
On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:19 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Michael Park <mc...@gmail.com>.
> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/tests/mesos.hpp, lines 866-874
> > <https://reviews.apache.org/r/31265/diff/8/?file=929838#file929838line866>
> >
> > An idea to avoid this: we could promote the `createAllocator` currently in `MasterAllocatorTest` to this file. Then we could do:
> >
> > ```
> > TestAllocator(
> > process::Owned<mesos::master::allocator::Allocator> realAllocator =
> > createAllocator<
> > mesos::internal::master::allocator::HierarchicalDRFAllocator>())
> > ```
Scratch that. I think we can do better. I think rather than `createAllocator` or `createTestAllocator`, a `create` factory function for `TestAllocator` fits nicely!
```cpp
class TestAllocator : public mesos::master::allocator::Allocator
{
public:
template <typename T = mesos::internal::master::allocator::HierarchicalDRFAllocator>
static TestAllocator create() {
// T represents the allocator type. It can be a default built-in
// allocator, or one provided by an allocator module.
Try<Allocator*> allocator = T::create();
CHECK_SOME(allocator);
// Wrap allocator instance in TestAllocator.
process::Owned<Allocator> realAllocator(CHECK_NOTNULL(allocator.get()));
return TestAllocator(realAllocator);
}
...
private:
// No need for default argument here.
TestAllocator(process::Owned<mesos::master::allocator::Allocator> realAllocator)
: real(realAllocator)
{
...
}
};
```
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review80445
-----------------------------------------------------------
On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:19 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/tests/mesos.hpp, lines 866-874
> > <https://reviews.apache.org/r/31265/diff/8/?file=929838#file929838line866>
> >
> > An idea to avoid this: we could promote the `createAllocator` currently in `MasterAllocatorTest` to this file. Then we could do:
> >
> > ```
> > TestAllocator(
> > process::Owned<mesos::master::allocator::Allocator> realAllocator =
> > createAllocator<
> > mesos::internal::master::allocator::HierarchicalDRFAllocator>())
> > ```
>
> Michael Park wrote:
> Scratch that. I think we can do better. I think rather than `createAllocator` or `createTestAllocator`, a `create` factory function for `TestAllocator` fits nicely!
>
> ```cpp
> class TestAllocator : public mesos::master::allocator::Allocator
> {
> public:
>
> template <typename T = mesos::internal::master::allocator::HierarchicalDRFAllocator>
> static TestAllocator create() {
> // T represents the allocator type. It can be a default built-in
> // allocator, or one provided by an allocator module.
> Try<Allocator*> allocator = T::create();
> CHECK_SOME(allocator);
>
> // Wrap allocator instance in TestAllocator.
> process::Owned<Allocator> realAllocator(CHECK_NOTNULL(allocator.get()));
> return TestAllocator(realAllocator);
> }
>
> ...
>
> private:
>
> // No need for default argument here.
> TestAllocator(process::Owned<mesos::master::allocator::Allocator> realAllocator)
> : real(realAllocator)
> {
> ...
> }
>
> };
> ```
The problem with the latter approach is that we won't be able to create `TestAllocator` instances on the heap: factory returns by value (which is not common for Mesos codebase), copy construction is not well-defined. I would like to let users decide where they want to have allocator instance.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review80445
-----------------------------------------------------------
On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:19 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/local/local.cpp, lines 141-145
> > <https://reviews.apache.org/r/31265/diff/8/?file=929832#file929832line141>
> >
> > What's the point of setting `_allocator` here? It looks like it only gets passed to the `new Master(...)` call.
> >
> > Can we just leave `_allocator` alone and do
> >
> > ```
> > if (allocator_.isError()) {
> > LOG(ERROR) << "Failed to create an instance of HierarchicalDRFAllocator: "
> > << allocator_.error();
> > allocator = NULL;
> > }
> >
> > // Save the instance for deleting later.
> > allocator = allocator_.get();
> > ```
> >
> > then pass `allocator` to `new Master` below?
`_allocator` is passed by non-const pointer, so theoretically we can modify caller's data. I can't find any code where this is the case, but I still don't feel like doing this drive-by change. Let's create a separate JIRA if you feel this should be fixed.
> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 77
> > <https://reviews.apache.org/r/31265/diff/8/?file=929836#file929836line77>
> >
> > I see this is the place where you wanted to use `ASSERT_SOME` but couldn't due to the [gtest limitation](https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement).
> >
> > One way to solve this I think is to use `SetUp/TearDown` rather than `Ctor/Dtor`.
> > Another way may be to use `CHECK_SOME` in place of `ASSERT_SOME`. I think this makes sense as well since success of `HierarchicalDRFAllocator::create` isn't actually a testing condition.
> >
> > Thoughts?
>
> Michael Park wrote:
> Actually, is it possible just use a `TestAllocator` constructed from `TestAllocator::create<>()` as suggested below?
Good idea!
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review80445
-----------------------------------------------------------
On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:19 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review80445
-----------------------------------------------------------
src/local/local.cpp
<https://reviews.apache.org/r/31265/#comment130361>
What's the point of setting `_allocator` here? It looks like it only gets passed to the `new Master(...)` call.
Can we just leave `_allocator` alone and do
```
if (allocator_.isError()) {
LOG(ERROR) << "Failed to create an instance of HierarchicalDRFAllocator: "
<< allocator_.error();
allocator = NULL;
}
// Save the instance for deleting later.
allocator = allocator_.get();
```
then pass `allocator` to `new Master` below?
src/tests/hierarchical_allocator_tests.cpp
<https://reviews.apache.org/r/31265/#comment130364>
I see this is the place where you wanted to use `ASSERT_SOME` but couldn't due to the [gtest limitation](https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement).
One way to solve this I think is to use `SetUp/TearDown` rather than `Ctor/Dtor`.
Another way may be to use `CHECK_SOME` in place of `ASSERT_SOME`. I think this makes sense as well since success of `HierarchicalDRFAllocator::create` isn't actually a testing condition.
Thoughts?
src/tests/mesos.hpp
<https://reviews.apache.org/r/31265/#comment130366>
An idea to avoid this: we could promote the `createAllocator` currently in `MasterAllocatorTest` to this file. Then we could do:
```
TestAllocator(
process::Owned<mesos::master::allocator::Allocator> realAllocator =
createAllocator<
mesos::internal::master::allocator::HierarchicalDRFAllocator>())
```
- Michael Park
On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:19 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review81038
-----------------------------------------------------------
Ship it!
Ship It!
- Niklas Nielsen
On April 21, 2015, 11:45 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 21, 2015, 11:45 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp af27a9bd8299cbff01e04b74db47c86bf247b908
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 0b564a74d3f04df46fe52fcbe1bf8d4d1e41c53c
> src/tests/mesos.hpp 7744df55a2a31446327da7bd2b16457e90711d22
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 21, 2015, 6:45 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Minor cleanup.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/examples/test_allocator_module.cpp PRE-CREATION
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/mesos/allocator.hpp af27a9bd8299cbff01e04b74db47c86bf247b908
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/hierarchical_allocator_tests.cpp 0b564a74d3f04df46fe52fcbe1bf8d4d1e41c53c
src/tests/mesos.hpp 7744df55a2a31446327da7bd2b16457e90711d22
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 21, 2015, 6:31 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Fail early, fail often.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/examples/test_allocator_module.cpp PRE-CREATION
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/mesos/allocator.hpp af27a9bd8299cbff01e04b74db47c86bf247b908
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/hierarchical_allocator_tests.cpp 0b564a74d3f04df46fe52fcbe1bf8d4d1e41c53c
src/tests/mesos.hpp 7744df55a2a31446327da7bd2b16457e90711d22
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 21, 2015, 6:03 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Improved variables naming.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/examples/test_allocator_module.cpp PRE-CREATION
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/mesos/allocator.hpp af27a9bd8299cbff01e04b74db47c86bf247b908
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/hierarchical_allocator_tests.cpp 0b564a74d3f04df46fe52fcbe1bf8d4d1e41c53c
src/tests/mesos.hpp 7744df55a2a31446327da7bd2b16457e90711d22
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 20, 2015, 1:50 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Refactored based on MPark's suggestions.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/examples/test_allocator_module.cpp PRE-CREATION
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/mesos/allocator.hpp af27a9bd8299cbff01e04b74db47c86bf247b908
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/hierarchical_allocator_tests.cpp 0b564a74d3f04df46fe52fcbe1bf8d4d1e41c53c
src/tests/mesos.hpp 7744df55a2a31446327da7bd2b16457e90711d22
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 15, 2015, 2:19 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Made c-tor private.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/examples/test_allocator_module.cpp PRE-CREATION
src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 7, 2015, 12:49 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
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 2, 2015, 7:59 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/allocator.hpp, line 47
> > <https://reviews.apache.org/r/31265/diff/6/?file=912110#file912110line47>
> >
> > Do you still want the constructor public?
I think there is no reason to hide the c-tor. I can imagine somebody creating an instance on the stack and do not see why we should forbid it. Same approach is used for example for `MesosContainerizer`.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review78711
-----------------------------------------------------------
On April 1, 2015, 2:28 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 2:28 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On April 2, 2015, 7:59 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/allocator.hpp, line 47
> > <https://reviews.apache.org/r/31265/diff/6/?file=912110#file912110line47>
> >
> > Do you still want the constructor public?
>
> Alexander Rukletsov wrote:
> I think there is no reason to hide the c-tor. I can imagine somebody creating an instance on the stack and do not see why we should forbid it. Same approach is used for example for `MesosContainerizer`.
On second thought I agree with you and will fix it.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review78711
-----------------------------------------------------------
On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:19 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/examples/test_allocator_module.cpp PRE-CREATION
> src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f
> src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec
> src/tests/hierarchical_allocator_tests.cpp 8861bf398e4bb17b0f74eab4f4af26202447ccef
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
> src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review78711
-----------------------------------------------------------
src/master/allocator/mesos/allocator.hpp
<https://reviews.apache.org/r/31265/#comment127667>
Do you still want the constructor public?
- Vinod Kone
On April 1, 2015, 2:28 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 2:28 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
> src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated April 1, 2015, 2:28 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
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated March 27, 2015, 4:27 p.m.)
Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
Changes
-------
Addressed MParks's comments.
Bugs: MESOS-2160
https://issues.apache.org/jira/browse/MESOS-2160
Repository: mesos
Description
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/tests/master_allocator_tests.cpp 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated March 23, 2015, 2:11 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
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/#review76679
-----------------------------------------------------------
For consistency, I would still root for Allocator::create() "factories". Try to take a look at master detector and contender :)
- Niklas Nielsen
On March 13, 2015, 2:50 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> -----------------------------------------------------------
>
> (Updated March 13, 2015, 2:50 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
> -------
>
> The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
>
> Diff: https://reviews.apache.org/r/31265/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 31265: Provided a factory for allocator in tests.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31265/
-----------------------------------------------------------
(Updated March 13, 2015, 9:50 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
-------
The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Diffs (updated)
-----
src/tests/master_allocator_tests.cpp a432d0207e1a92532a495bf9ad2826414ee4f6f0
Diff: https://reviews.apache.org/r/31265/diff/
Testing
-------
make check (Mac OS 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov