You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2014/12/31 00:57:32 UTC

Review Request 29496: Added example hook module.

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

Review request for mesos.


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs
-----

  src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29496/#review67963
-----------------------------------------------------------



src/examples/test_hook_module.cpp
<https://reviews.apache.org/r/29496/#comment112046>

    s/temp/temporary/



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/29496/#comment112048>

    kill newline



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/29496/#comment112050>

    Can you add that you are testing that the test hook hangs a new label off the taskinfo?



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/29496/#comment112051>

    Also here; can you add that you expect one hook to create a temporary file and the other to clean it up?


- Niklas Nielsen


On Jan. 13, 2015, 10:37 a.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29496/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 10:37 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2060
>     https://issues.apache.org/jira/browse/MESOS-2060
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
>   src/examples/test_hook_module.cpp PRE-CREATION 
>   src/tests/hook_tests.cpp PRE-CREATION 
>   src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
>   src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 
> 
> Diff: https://reviews.apache.org/r/29496/diff/
> 
> 
> Testing
> -------
> 
> make  check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 29496: Added example hook module.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29496/#review68004
-----------------------------------------------------------

Ship it!


Ship It!

- Niklas Nielsen


On Jan. 13, 2015, 5:05 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29496/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 5:05 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2060
>     https://issues.apache.org/jira/browse/MESOS-2060
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
>   src/examples/test_hook_module.cpp PRE-CREATION 
>   src/tests/hook_tests.cpp PRE-CREATION 
>   src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
>   src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 
> 
> Diff: https://reviews.apache.org/r/29496/diff/
> 
> 
> Testing
> -------
> 
> make  check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 14, 2015, 4:22 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
-------

Replaced SetUp/TearDown with constructor/destructor; causes make check to succeed.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 9:21 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
-------

Added SetUp/TearDown() for hook tests.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 9:09 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
-------

added unload() and more tests.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 8:05 p.m.)


Review request for mesos and Niklas Nielsen.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 8:05 p.m.)


Review request for mesos.


Changes
-------

minor fixes.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 7:49 p.m.)


Review request for mesos.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 7:36 p.m.)


Review request for mesos.


Changes
-------

Addressed Nik's comments.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 13, 2015, 1:37 p.m.)


Review request for mesos.


Changes
-------

addressed nik's comments.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs (updated)
-----

  src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya


Re: Review Request 29496: Added example hook module.

Posted by Kapil Arya <ka...@mesosphere.io>.

> On Jan. 6, 2015, 8:55 a.m., Niklas Nielsen wrote:
> > src/examples/test_hook_module.cpp, lines 38-40
> > <https://reviews.apache.org/r/29496/diff/1/?file=803967#file803967line38>
> >
> >     I wonder if we can consolidate these strings with the ones in the test code. Have you tried to extern them from the test and grab them here?

The strings are defined in two separate objects and it's not trivial to share them.  Especially, when we want to test loading one object without necessarily using the other.  However, I have now added a comment about the source and that these should be kept in sync.


> On Jan. 6, 2015, 8:55 a.m., Niklas Nielsen wrote:
> > src/tests/hook_tests.cpp, lines 117-118
> > <https://reviews.apache.org/r/29496/diff/1/?file=803968#file803968line117>
> >
> >     I am not sure I understand why you are doing this. Can you expand a little bit?

We need to read the taskInfo to verify the presence of labels inserted by the label decorator hook.


- Kapil


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


On Jan. 13, 2015, 1:37 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29496/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 1:37 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2060
>     https://issues.apache.org/jira/browse/MESOS-2060
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 
>   src/examples/test_hook_module.cpp PRE-CREATION 
>   src/tests/hook_tests.cpp PRE-CREATION 
>   src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
>   src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 
> 
> Diff: https://reviews.apache.org/r/29496/diff/
> 
> 
> Testing
> -------
> 
> make  check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 29496: Added example hook module.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29496/#review66823
-----------------------------------------------------------



src/examples/test_hook_module.cpp
<https://reviews.apache.org/r/29496/#comment110479>

    I wonder if we can consolidate these strings with the ones in the test code. Have you tried to extern them from the test and grab them here?



src/examples/test_hook_module.cpp
<https://reviews.apache.org/r/29496/#comment110478>

    Mind adding a comment on what the test env decorator is doing and how you will validate it in together with masterLaunchTaskLabelDecorator and slaveRemoveExecutorHook?



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/29496/#comment110477>

    Test comment please :-)
    
    Also, it is a bit hard following what's going on. Can you add some comments to the test body too?



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/29496/#comment110480>

    I am not sure I understand why you are doing this. Can you expand a little bit?



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/29496/#comment110476>

    Same here


- Niklas Nielsen


On Jan. 5, 2015, 2:42 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29496/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2015, 2:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2060
>     https://issues.apache.org/jira/browse/MESOS-2060
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586 
>   src/examples/test_hook_module.cpp PRE-CREATION 
>   src/tests/hook_tests.cpp PRE-CREATION 
>   src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
>   src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 
> 
> Diff: https://reviews.apache.org/r/29496/diff/
> 
> 
> Testing
> -------
> 
> make  check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 29496: Added example hook module.

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

(Updated Jan. 5, 2015, 5:42 p.m.)


Review request for mesos.


Changes
-------

Added jira ticket.


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


Repository: mesos-git


Description
-------

This module provides hooks for master label decorator, slave executor environment decorator and slave remove executor.  A couple of test cases are also provided to verify the hooks from this module.


Diffs
-----

  src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586 
  src/examples/test_hook_module.cpp PRE-CREATION 
  src/tests/hook_tests.cpp PRE-CREATION 
  src/tests/module.hpp bc1a37df7f95e945363418429fef4d090907c73a 
  src/tests/module.cpp 6cec1cbafe4750cff005191c51ce7c08149c18f4 

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


Testing
-------

make  check


Thanks,

Kapil Arya