You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@mesos.apache.org by "Alexander Rojas (JIRA)" <ji...@apache.org> on 2015/08/10 14:37:45 UTC
[jira] [Commented] (MESOS-3248) Allow developers the option to pass
parameters to modules programatically
[ https://issues.apache.org/jira/browse/MESOS-3248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14680040#comment-14680040 ]
Alexander Rojas commented on MESOS-3248:
----------------------------------------
My solution for this problem will look as follows:
Under this proposal, the first step is to overload the factory method {{create()}} of the {{ModuleManager}} by adding the following:
{code}
template <typename T>
static Try<T*> ModuleManager<T>::create(
const std::string& moduleName,
const Parameters& params)
{
synchronized (mutex) {
if (!moduleBases.contains(moduleName)) {
return Error(
"Module '" + moduleName + "' unknown");
}
Module<T>* module = (Module<T>*) moduleBases[moduleName];
if (module->create == NULL) {
return Error(
"Error creating module instance for '" + moduleName + "': "
"create() method not found");
}
std::string expectedKind = kind<T>();
if (expectedKind != module->kind) {
return Error(
"Error creating module instance for '" + moduleName + "': "
"module is of kind '" + module->kind + "', but the requested "
"kind is '" + expectedKind + "'");
}
T* instance = module->create(params);
if (instance == NULL) {
return Error("Error creating Module instance for '" + moduleName + "'");
}
return instance;
}
}
{code}
Then the original {{create()}} method can be simplified to:
{code}
template <typename T>
static Try<T*> ModuleManager<T>::create(const std::string& moduleName)
{
return create(moduleName, moduleParameters[moduleName]);
}
{code}
With this we can keep a simple version of {{ParameterFoo}}:
{code}
class ParameterFoo {
public:
Try<Foo*> create(int i) {
return new ParameterFoo(i);
}
Try<Foo*> create(const Parameters& params) {
Option<int> param = None;
std::size_t error;
for (const auto& param : params.parameter()) {
if (param.key() == "i") {
param = std::stoi(param.value(), &error);
if (error == 0) {
return Error("Could not parse parameters");
}
}
}
if (param.isNone()) {
return Error("Wrong type given in the parameters");
}
return create(param.get());
}
ParameterFoo(int i) : i_(i) {}
virtual Future<int> hello() {
return i;
}
private:
int i_;
};
{code}
Some changes in {{tests::Module}} will be needed, adding an overload for
create:
{code}
template<typename T>
static Try<T*> tests::Module<T>::create(const Parameters& params)
{
Try<std::string> moduleName = getModuleName(N);
if (moduleName.isError()) {
return Error(moduleName.error());
}
return mesos::modules::ModuleManager::create<T>(moduleName.get(), params);
}
{code}
The test can thus be written as:
{code}
typedef ::testing::Types<ParameterFoo,
tests::Module<Foo, TestParameterFoo>>
FooTestTypes;
TYPED_TEST_CASE(FooTest, FooTestTypes);
TYPED_TEST(FooTest, ATest)
{
int fooValue = 1;
// This part can go in the fixture set up
Parameters params;
Parameter* param = params.add_parameter();
param->set_key("i");
param->set_value(std::to_string(fooValue));
Try<Foo*> foo = TypeParam::create(params);
ASSERT_SOME(foo);
AWAIT_CHECK_EQUAL(foo.get()->hello(), fooValue);
}
{code}
> Allow developers the option to pass parameters to modules programatically
> -------------------------------------------------------------------------
>
> Key: MESOS-3248
> URL: https://issues.apache.org/jira/browse/MESOS-3248
> Project: Mesos
> Issue Type: Bug
> Components: modules
> Reporter: Alexander Rojas
> Labels: mesosphere
>
> h1.Introduction
> As it stands right now, default implementations of modularized components are required to have a non parametrized {{create()}} static method. This allows to write tests which can cover default implementations and modules based on these default implementations on a uniform way.
> For example, with the interface {{Foo}}:
> {code}
> class Foo {
> public:
> virtual ~Foo() {}
> virtual Future<int> hello() = 0;
> protected:
> Foo() {}
> };
> {code}
> With a default implementation:
> {code}
> class LocalFoo {
> public:
> Try<Foo*> create() {
> return new Foo;
> }
> virtual Future<int> hello() {
> return 1;
> }
> };
> {code}
> This allows to create typed tests which look as following:
> {code}
> typedef ::testing::Types<LocalFoo,
> tests::Module<Foo, TestLocalFoo>>
> FooTestTypes;
> TYPED_TEST_CASE(FooTest, FooTestTypes);
> TYPED_TEST(FooTest, ATest)
> {
> Try<Foo*> foo = TypeParam::create();
> ASSERT_SOME(foo);
> AWAIT_CHECK_EQUAL(foo.get()->hello(), 1);
> }
> {code}
> The test will be applied to each of types in the template parameters of {{FooTestTypes}}. This allows to test different implementation of an interface. In our code, it tests default implementations and a module which uses the same default implementation.
> The class {{tests::Module<typename T, ModuleID N>}} needs a little explanation, it is a wrapper around {{ModuleManager}} which allows the tests to encode information about the requested module in the type itself instead of passing a string to the factory method. The wrapper around create, the real important method looks as follows:
> {code}
> template<typename T, ModuleID N>
> static Try<T*> test::Module<T, N>::create()
> {
> Try<std::string> moduleName = getModuleName(N);
> if (moduleName.isError()) {
> return Error(moduleName.error());
> }
> return mesos::modules::ModuleManager::create<T>(moduleName.get());
> }
> {code}
> h1.The Problem
> Consider the following implementation of {{Foo}}:
> {code}
> class ParameterFoo {
> public:
> Try<Foo*> create(int i) {
> return new ParameterFoo(i);
> }
> ParameterFoo(int i) : i_(i) {}
> virtual Future<int> hello() {
> return i;
> }
> private:
> int i_;
> };
> {code}
> As it can be seen, this implementation cannot be used as a default implementation since its create API does not match the one of {{test::Module<>}}, however it is a common situation to require initialization parameters for objects, however this constraint forces default implementations of modularized components to have default constructors.
> Module only implementations are allowed to have constructor parameters, since the actual signature of their factory method is:
> {code}
> template<typename T>
> T* Module<T>::create(const Parameters& params);
> {code}
> where parameters is just an array of key-value string pairs whose interpretation is left to the specific module. However, this call is wrapped by
> {{ModuleManager}} which only allows module parameters to be passed from the command line and do not offer a programmatic way to feed construction parameters to modules.
> h1.The Ugly Workaround
> With the requirement of a default constructor and parameters devoid {{create()}} factory function, a common pattern has been introduced to feed construction parameters into default implementation, this leads to adding an {{initialize()}} call to the public interface, which will have {{Foo}} become:
> {code}
> class Foo {
> public:
> virtual ~Foo() {}
> virtual Try<Nothing> initialize(Option<int> i) = 0;
> virtual Future<int> hello() = 0;
> protected:
> Foo() {}
> };
> {code}
> {{ParameterFoo}} will thus look as follows:
> {code}
> class ParameterFoo {
> public:
> Try<Foo*> create() {
> return new ParameterFoo;
> }
> ParameterFoo() : i_(None()) {}
> virtual Try<Nothing> initialize(Option<int> i) {
> if (i.isNone()) {
> return Error("Need value to initialize");
> }
> i_ = i;
> return Nothing;
> }
> virtual Future<int> hello() {
> if (i_.isNone()) {
> return Future<int>::failure("Not initialized");
> }
> return i_.get();
> }
> private:
> Option<int> i_;
> };
> {code}
> Look that this {{initialize()}} method now has to be implemented by all descendants of {{Foo}}, even if there's a {{DatabaseFoo}} which takes is
> return value for {{hello()}} from a DB, it will need to support {{int}} as an initialization parameter.
> The problem is more severe the more specific the parameter to {{initialize()}} is. For example, if there is a very complex structure implementing ACLs, all implementations of an authorizer will need to import this structure even if they can completely ignore it.
> In the {{Foo}} example if {{ParameterFoo}} were to become the default implementation of {{Foo}}, the tests would look as follows:
> {code}
> typedef ::testing::Types<ParameterFoo,
> tests::Module<Foo, TestParameterFoo>>
> FooTestTypes;
> TYPED_TEST_CASE(FooTest, FooTestTypes);
> TYPED_TEST(FooTest, ATest)
> {
> Try<Foo*> foo = TypeParam::create();
> ASSERT_SOME(foo);
> int fooValue = 1;
> foo.get()->initialize(fooValue);
> AWAIT_CHECK_EQUAL(foo.get()->hello(), fooValue);
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)