> 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 > >
