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

2016-07-09 Thread Joris Van Remoortere 

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On Feb. 13, 2016, 6: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, 6: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.

2016-02-27 Thread Kapil Arya 

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


Pls replace it with `'.



include/mesos/module/anonymous.hpp (line 58)


Remove double backquote.



include/mesos/module/anonymous.hpp (line 59)


Indent by four spaces.



src/examples/test_anonymous_module.hpp (lines 22 - 26)


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



src/examples/test_anonymous_module.hpp (lines 31 - 36)


Fix Indentation.



src/examples/test_anonymous_module.hpp (lines 32 - 34)


Drop `virtual`?



src/examples/test_anonymous_module.cpp (line 49)


Kill the extra space after return type.



src/examples/test_anonymous_module.cpp (line 68)


May be drop the `-`?



src/examples/test_anonymous_module.cpp (line 70)


Kill extra newline.



src/examples/test_anonymous_module.cpp (line 77)


Add another newline.



src/examples/test_anonymous_module.cpp (line 83)


Add another newline.



src/examples/test_anonymous_module.cpp (lines 86 - 92)


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



src/examples/test_anonymous_module.cpp (lines 93 - 94)


Kill extra newlines.



src/slave/main.cpp (lines 298 - 299)


Update the comment to reflect the initialization functionality.



src/slave/main.cpp (line 307)


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



src/tests/anonymous_tests.cpp (lines 17 - 18)


Not used.



src/tests/anonymous_tests.cpp (line 51)


Brace on a newline.



src/tests/anonymous_tests.cpp (lines 53 - 58)


Indent by two spaces.



src/tests/anonymous_tests.cpp (line 71)


Drop `std::`.



src/tests/anonymous_tests.cpp (lines 93 - 99)


Consolidate line 93 and 99.



src/tests/anonymous_tests.cpp (lines 95 - 102)


Fix indentation



src/tests/anonymous_tests.cpp (line 97)


s/ASSERT_SOME/EXPECT_SOME/



src/tests/anonymous_tests.cpp (line 102)


Replace with `EXPECT_SOME(result)`.



src/tests/anonymous_tests.cpp (line 106)


s/ASSERT/EXPECT/



src/tests/module.hpp (line 48)


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

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

2016-02-26 Thread Kapil Arya 

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

2016-02-12 Thread Marco Massenzio 

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