You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Marco Massenzio <m....@gmail.com> on 2016/01/04 21:16:35 UTC

Re: Review Request 41760: Add initialization method to Anonymous module

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

(Updated Jan. 4, 2016, 8:16 p.m.)


Review request for mesos, Anand Mazumdar and Kapil Arya.


Summary (updated)
-----------------

Add initialization method to Anonymous module


Bugs: MESOS-4253
    https://issues.apache.org/jira/browse/MESOS-4253


Repository: mesos


Description
-------

Jira: [MESOS-4253](https://issues.apache.org/jira/browse/MESOS-4253)

To provide a basic "context" to Anonymous modules, we pass in the `BaseFlags`.

This tries to be backward compatible with existing modules, so a no-op (`virtual`) implementation of the method is provided in the base class.
Currently the code is only implemented in the Agent's `main()` method, but if deemed correct, adding it to Master is trivial.


Diffs
-----

  include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
  src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
  src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
  src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
  src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
  src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22 
  src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0 

Diff: https://reviews.apache.org/r/41760/diff/


Testing
-------

Unit tests pass.

Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
```
I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
```


Thanks,

Marco Massenzio


Re: Review Request 41760: Add initialization method to Anonymous module

Posted by Marco Massenzio <m....@gmail.com>.

> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > I am little concerned about `Flags` being passed to the module. Since there is no visibility about the allowed master/agent flags from the module's perspective, how does it cope with the changes to master/slave flags? Would we want to expose available master/slave flags as well in include/mesos? It's not quite clear what the correct representation would be.
> 
> Marco Massenzio wrote:
>     Hey Kapil,
>     
>     thanks for review!
>     I'm in the middle of something, but I'll get round to it this weekend - it all seems very agreeable at a quick glance.
>     
>     I'll reply with more detail, but in the meantime, please note that teh idea was *not* expose the master/agent flags as types - the module simply iterates over the map (and getting the string values, not the actual typed objects); but I'm assuming that the flags' names will be from *fairly* to *very* stable, so the risk is rather low?
>     
>     I think that exposing the flags in an include module would actually make matters worse, as it would cause backward compatibility issues and would impact the ability of Mesos to move forward wrt existing modules?
>     
>     In any event, the way to cope with change would be as usual: for the *required* ones the module would have to fail (great suggestion to have a `Try` returned by the initialization) and for the *optional* ones, it will just have to make do without (possibly emitting warnings).

So, as a practical example, this is how it would be used in a module:
```
  virtual void initialize(const flags::FlagsBase& flags)
  {
    string workDir;
    string sandboxDir;

    LOG(INFO) << "Executing init() for module; parsing runtime flags";
    for(auto flag : flags) {
      string name = flag.first;
      Option<string> value = flag.second.stringify(flags);
      if (name == "work_dir" && value.isSome()) {
        workDir = value.get();
      } else if (name == "sandbox_directory" && value.isSome()) {
        sandboxDir = value.get();
      }
    }
    LOG(INFO) << "Configured work dir to [" << workDir
              << "] and Sandbox dir to [" << sandboxDir << "]";
    process = new RemoteExecutionProcess(workDir, sandboxDir);
    spawn(process);
  }
```

This would only "break" if the names of the flags were to change - this would be, I believe, extremely unlikely?


> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > include/mesos/module/anonymous.hpp, line 59
> > <https://reviews.apache.org/r/41760/diff/2/?file=1180449#file1180449line59>
> >
> >     I am wondering if we should allow the module to indicate an error during initialization. Should the return type be `Try<Nothing>` instead?

Absolutely!
done.


> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > src/examples/test_anonymous_module.hpp, lines 25-48
> > <https://reviews.apache.org/r/41760/diff/2/?file=1180450#file1180450line25>
> >
> >     Let's move this to the cpp file as with the other modules.

Done - please note that we still need to leave the class definition in the header file, as the test needs to cast the module object to verify it was correctly created.

moved ``initialize()`` to the .cpp file, though.


> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > src/examples/test_anonymous_module.cpp, lines 56-60
> > <https://reviews.apache.org/r/41760/diff/2/?file=1180451#file1180451line56>
> >
> >     Do we also need a destructor just like in TestAnonymous?

Done; however TestAnonymous' destructor just logs, not a very useful destructor?
We don't allocate memory/resources that need freeing.

Also, in the current code, anon modules' destructors are never called (I was planning to implement that - there is a TODO from benh - once we got this patch reviewed).


> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > src/slave/main.cpp, lines 275-278
> > <https://reviews.apache.org/r/41760/diff/2/?file=1180452#file1180452line275>
> >
> >     Just add a CHECK here and remove the if-else. The module manager returns `Error` for NULL.

Done.
I've also added a check on the Try returned by initialize() - I am also following the same pattern as `create()`:
```
    if (create.isError()) {
      EXIT(EXIT_FAILURE)
        << "Failed to create anonymous module named '" << name << "'";
    }
```
however, I'm not entirely sure we should crash the Agent (or Master, for that matter) if an anon module cannot be initialized?
Are you ok with that?


> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > src/tests/anonymous_tests.cpp, line 106
> > <https://reviews.apache.org/r/41760/diff/2/?file=1180453#file1180453line106>
> >
> >     Is there a way to validate this typecast?

Not that I'm aware of (in Python you'd do `isinstance()` ;D )
I mean, we could use a `try / catch`, but those are not used in Mesos, right?

In any event, this is a test - if the cast fails, it means there's an error somewhere, the test crashes and we catch the error: everyone is happy?


- Marco


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


On Jan. 4, 2016, 8:16 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41760/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Kapil Arya.
> 
> 
> Bugs: MESOS-4253
>     https://issues.apache.org/jira/browse/MESOS-4253
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: [MESOS-4253](https://issues.apache.org/jira/browse/MESOS-4253)
> 
> To provide a basic "context" to Anonymous modules, we pass in the `BaseFlags`.
> 
> This tries to be backward compatible with existing modules, so a no-op (`virtual`) implementation of the method is provided in the base class.
> Currently the code is only implemented in the Agent's `main()` method, but if deemed correct, adding it to Master is trivial.
> 
> 
> Diffs
> -----
> 
>   include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
>   src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
>   src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
>   src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22 
>   src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0 
> 
> Diff: https://reviews.apache.org/r/41760/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
> ```
> I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
> I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
> ```
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 41760: Add initialization method to Anonymous module

Posted by Marco Massenzio <m....@gmail.com>.

> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > I am little concerned about `Flags` being passed to the module. Since there is no visibility about the allowed master/agent flags from the module's perspective, how does it cope with the changes to master/slave flags? Would we want to expose available master/slave flags as well in include/mesos? It's not quite clear what the correct representation would be.

Hey Kapil,

thanks for review!
I'm in the middle of something, but I'll get round to it this weekend - it all seems very agreeable at a quick glance.

I'll reply with more detail, but in the meantime, please note that teh idea was *not* expose the master/agent flags as types - the module simply iterates over the map (and getting the string values, not the actual typed objects); but I'm assuming that the flags' names will be from *fairly* to *very* stable, so the risk is rather low?

I think that exposing the flags in an include module would actually make matters worse, as it would cause backward compatibility issues and would impact the ability of Mesos to move forward wrt existing modules?

In any event, the way to cope with change would be as usual: for the *required* ones the module would have to fail (great suggestion to have a `Try` returned by the initialization) and for the *optional* ones, it will just have to make do without (possibly emitting warnings).


- Marco


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


On Jan. 4, 2016, 8:16 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41760/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Kapil Arya.
> 
> 
> Bugs: MESOS-4253
>     https://issues.apache.org/jira/browse/MESOS-4253
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: [MESOS-4253](https://issues.apache.org/jira/browse/MESOS-4253)
> 
> To provide a basic "context" to Anonymous modules, we pass in the `BaseFlags`.
> 
> This tries to be backward compatible with existing modules, so a no-op (`virtual`) implementation of the method is provided in the base class.
> Currently the code is only implemented in the Agent's `main()` method, but if deemed correct, adding it to Master is trivial.
> 
> 
> Diffs
> -----
> 
>   include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
>   src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
>   src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
>   src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22 
>   src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0 
> 
> Diff: https://reviews.apache.org/r/41760/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
> ```
> I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
> I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
> ```
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 41760: Add initialization method to Anonymous module

Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41760/#review118242
-----------------------------------------------------------



I am little concerned about `Flags` being passed to the module. Since there is no visibility about the allowed master/agent flags from the module's perspective, how does it cope with the changes to master/slave flags? Would we want to expose available master/slave flags as well in include/mesos? It's not quite clear what the correct representation would be.


include/mesos/module/anonymous.hpp (line 59)
<https://reviews.apache.org/r/41760/#comment179468>

    I am wondering if we should allow the module to indicate an error during initialization. Should the return type be `Try<Nothing>` instead?



src/examples/test_anonymous_module.hpp (lines 25 - 48)
<https://reviews.apache.org/r/41760/#comment179462>

    Let's move this to the cpp file as with the other modules.



src/examples/test_anonymous_module.cpp (lines 54 - 58)
<https://reviews.apache.org/r/41760/#comment179463>

    Do we also need a destructor just like in TestAnonymous?



src/examples/test_anonymous_module.cpp (line 82)
<https://reviews.apache.org/r/41760/#comment179464>

    Update description?



src/slave/main.cpp (lines 275 - 278)
<https://reviews.apache.org/r/41760/#comment179465>

    Just add a CHECK here and remove the if-else. The module manager returns `Error` for NULL.



src/tests/anonymous_tests.cpp (lines 95 - 102)
<https://reviews.apache.org/r/41760/#comment179466>

    Please do an explicit initialization:
    
    Try<Anonymous*> module = ModuleManager::create<Anonymous>("org_apache_mesos_TestIni...");



src/tests/anonymous_tests.cpp (line 106)
<https://reviews.apache.org/r/41760/#comment179467>

    Is there a way to validate this typecast?



src/tests/anonymous_tests.cpp (lines 106 - 107)
<https://reviews.apache.org/r/41760/#comment179469>

    Can we just modify the module to fail during initialization if the required flags are not present and not do this check here?


- Kapil Arya


On Jan. 4, 2016, 3:16 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41760/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 3:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Kapil Arya.
> 
> 
> Bugs: MESOS-4253
>     https://issues.apache.org/jira/browse/MESOS-4253
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: [MESOS-4253](https://issues.apache.org/jira/browse/MESOS-4253)
> 
> To provide a basic "context" to Anonymous modules, we pass in the `BaseFlags`.
> 
> This tries to be backward compatible with existing modules, so a no-op (`virtual`) implementation of the method is provided in the base class.
> Currently the code is only implemented in the Agent's `main()` method, but if deemed correct, adding it to Master is trivial.
> 
> 
> Diffs
> -----
> 
>   include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
>   src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
>   src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
>   src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22 
>   src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0 
> 
> Diff: https://reviews.apache.org/r/41760/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
> ```
> I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
> I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
> ```
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 41760: Add initialization method to Anonymous class.

Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41760/#review121102
-----------------------------------------------------------



Hi Marco,

Sorry for the delayed response. While I already gave it a ship-it from the functionality point of view, there are still some style issues to be addressed before this can be merged.

Also, can you separate out the changes to slave/ into a separate review just like you have for the master/ changes?


include/mesos/module/anonymous.hpp (line 23)
<https://reviews.apache.org/r/41760/#comment182694>

    Pls replace it with `<stout/flags.hpp>'.



include/mesos/module/anonymous.hpp (line 58)
<https://reviews.apache.org/r/41760/#comment182695>

    Remove double backquote.



include/mesos/module/anonymous.hpp (line 59)
<https://reviews.apache.org/r/41760/#comment182696>

    Indent by four spaces.



src/examples/test_anonymous_module.hpp (lines 22 - 26)
<https://reviews.apache.org/r/41760/#comment182697>

    Move `using ...` to top. Kill the newline.



src/examples/test_anonymous_module.hpp (lines 31 - 36)
<https://reviews.apache.org/r/41760/#comment182709>

    Fix Indentation.



src/examples/test_anonymous_module.hpp (lines 32 - 34)
<https://reviews.apache.org/r/41760/#comment182698>

    Drop `virtual`?



src/examples/test_anonymous_module.cpp (line 49)
<https://reviews.apache.org/r/41760/#comment182710>

    Kill the extra space after return type.



src/examples/test_anonymous_module.cpp (line 68)
<https://reviews.apache.org/r/41760/#comment182711>

    May be drop the `-`?



src/examples/test_anonymous_module.cpp (line 70)
<https://reviews.apache.org/r/41760/#comment182712>

    Kill extra newline.



src/examples/test_anonymous_module.cpp (line 77)
<https://reviews.apache.org/r/41760/#comment182713>

    Add another newline.



src/examples/test_anonymous_module.cpp (line 83)
<https://reviews.apache.org/r/41760/#comment182714>

    Add another newline.



src/examples/test_anonymous_module.cpp (lines 86 - 92)
<https://reviews.apache.org/r/41760/#comment182716>

    Not yours, but could you fix the indentation here too?



src/examples/test_anonymous_module.cpp (lines 93 - 94)
<https://reviews.apache.org/r/41760/#comment182715>

    Kill extra newlines.



src/slave/main.cpp (lines 298 - 299)
<https://reviews.apache.org/r/41760/#comment182717>

    Update the comment to reflect the initialization functionality.



src/slave/main.cpp (line 307)
<https://reviews.apache.org/r/41760/#comment182718>

    Drop this log message. This is redundant with the one above.



src/tests/anonymous_tests.cpp (lines 17 - 18)
<https://reviews.apache.org/r/41760/#comment182701>

    Not used.



src/tests/anonymous_tests.cpp (line 51)
<https://reviews.apache.org/r/41760/#comment182700>

    Brace on a newline.



src/tests/anonymous_tests.cpp (lines 53 - 58)
<https://reviews.apache.org/r/41760/#comment182699>

    Indent by two spaces.



src/tests/anonymous_tests.cpp (line 71)
<https://reviews.apache.org/r/41760/#comment182702>

    Drop `std::`.



src/tests/anonymous_tests.cpp (lines 93 - 99)
<https://reviews.apache.org/r/41760/#comment182704>

    Consolidate line 93 and 99.



src/tests/anonymous_tests.cpp (lines 95 - 102)
<https://reviews.apache.org/r/41760/#comment182703>

    Fix indentation



src/tests/anonymous_tests.cpp (line 97)
<https://reviews.apache.org/r/41760/#comment182705>

    s/ASSERT_SOME/EXPECT_SOME/



src/tests/anonymous_tests.cpp (line 102)
<https://reviews.apache.org/r/41760/#comment182706>

    Replace with `EXPECT_SOME(result)`.



src/tests/anonymous_tests.cpp (line 106)
<https://reviews.apache.org/r/41760/#comment182707>

    s/ASSERT/EXPECT/



src/tests/module.hpp (line 48)
<https://reviews.apache.org/r/41760/#comment182708>

    s/TestInitModule/TestInitAnonymous/
    
    Here and everywhere else.


- Kapil Arya


On Feb. 13, 2016, 1:24 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41760/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Kapil Arya.
> 
> 
> Bugs: MESOS-4253
>     https://issues.apache.org/jira/browse/MESOS-4253
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-4253
> 
> To provide a basic "context" to anonymous modules, we provide
> an `initialize(const FlagsBase&)` method that will be invoked
> shortly after creation of the module class.
> 
> 
> Diffs
> -----
> 
>   include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
>   src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
>   src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
>   src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
>   src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
>   src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 
> 
> Diff: https://reviews.apache.org/r/41760/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
> ```
> I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
> I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
> ```
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 41760: Add initialization method to Anonymous class.

Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41760/#review121070
-----------------------------------------------------------


Ship it!




Ship It!

- Kapil Arya


On Feb. 13, 2016, 1:24 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41760/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Kapil Arya.
> 
> 
> Bugs: MESOS-4253
>     https://issues.apache.org/jira/browse/MESOS-4253
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-4253
> 
> To provide a basic "context" to anonymous modules, we provide
> an `initialize(const FlagsBase&)` method that will be invoked
> shortly after creation of the module class.
> 
> 
> Diffs
> -----
> 
>   include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
>   src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
>   src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
>   src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
>   src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
>   src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 
> 
> Diff: https://reviews.apache.org/r/41760/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
> ```
> I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
> I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
> ```
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 41760: Add initialization method to Anonymous class.

Posted by Marco Massenzio <m....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41760/
-----------------------------------------------------------

(Updated Feb. 13, 2016, 6:24 a.m.)


Review request for mesos, Anand Mazumdar and Kapil Arya.


Changes
-------

Addressed review's comment


Summary (updated)
-----------------

Add initialization method to Anonymous class.


Bugs: MESOS-4253
    https://issues.apache.org/jira/browse/MESOS-4253


Repository: mesos


Description (updated)
-------

Jira: MESOS-4253

To provide a basic "context" to anonymous modules, we provide
an `initialize(const FlagsBase&)` method that will be invoked
shortly after creation of the module class.


Diffs (updated)
-----

  include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
  src/examples/test_anonymous_module.hpp 3da33a42f04b7421cee8fbb85e29b66e352f5894 
  src/examples/test_anonymous_module.cpp dd291cff3b5d47337e371cd2c1082fd6716af3fc 
  src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
  src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 

Diff: https://reviews.apache.org/r/41760/diff/


Testing
-------

Unit tests pass.

Also implemented in the [execute-module](http://github.com/massenz/execute-module) - and it works there too:
```
I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 'com_alertavert_mesos_RemoteExecution'
I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 'com_alertavert_mesos_RemoteExecution'
I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() for module; parsing runtime flags
I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
```


Thanks,

Marco Massenzio