Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

2015-08-03 Thread Niklas Nielsen 

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



include/mesos/hook.hpp (lines 89 - 90)


The executor id should be in the task status 
(https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L973) 
have you run into it not being available?


- Niklas Nielsen


On July 31, 2015, 9:24 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37007/
> ---
> 
> (Updated July 31, 2015, 9:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3016
> https://issues.apache.org/jira/browse/MESOS-3016
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, only FrameworkID and TaskID are sent to the hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b 
>   src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf 
>   src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 
>   src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 
>   src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
> 
> Diff: https://reviews.apache.org/r/37007/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

2015-07-31 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37007]

All tests passed.

- Mesos ReviewBot


On Aug. 1, 2015, 4:24 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37007/
> ---
> 
> (Updated Aug. 1, 2015, 4:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3016
> https://issues.apache.org/jira/browse/MESOS-3016
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, only FrameworkID and TaskID are sent to the hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b 
>   src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf 
>   src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 
>   src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 
>   src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
> 
> Diff: https://reviews.apache.org/r/37007/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

2015-07-31 Thread Kapil Arya 


> On July 31, 2015, 10:22 p.m., Michael Park wrote:
> > src/slave/slave.cpp, lines 2740-2743
> > 
> >
> > Why did this have to move? I guess we want the `update` within the `if 
> > (executor == NULL)` statement block to have the old value?

I coule be off, but I think a hook would have already been called for the 
previous update, so we don't need to call it again. Further, the hook is pretty 
much useless without the executorId, as it is hard for a module to figure out 
the correct context with just frameworkId and taskId.


- Kapil


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


On Aug. 1, 2015, 12:24 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37007/
> ---
> 
> (Updated Aug. 1, 2015, 12:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3016
> https://issues.apache.org/jira/browse/MESOS-3016
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, only FrameworkID and TaskID are sent to the hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b 
>   src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf 
>   src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 
>   src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 
>   src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
> 
> Diff: https://reviews.apache.org/r/37007/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

2015-07-31 Thread Kapil Arya 

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

(Updated Aug. 1, 2015, 12:24 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Niklas Nielsen.


Changes
---

fixed typo.


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


Repository: mesos


Description
---

Currently, only FrameworkID and TaskID are sent to the hook.


Diffs (updated)
-

  include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b 
  src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf 
  src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 
  src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 
  src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 

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


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

2015-07-31 Thread Michael Park 

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



include/mesos/hook.hpp (line 89)


`s/executorID/executorId/` -- here and below.



src/slave/slave.cpp 


Why did this have to move? I guess we want the `update` within the `if 
(executor == NULL)` statement block to have the old value?


- Michael Park


On July 31, 2015, 11:27 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37007/
> ---
> 
> (Updated July 31, 2015, 11:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3016
> https://issues.apache.org/jira/browse/MESOS-3016
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, only FrameworkID and TaskID are sent to the hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b 
>   src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf 
>   src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 
>   src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 
>   src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
> 
> Diff: https://reviews.apache.org/r/37007/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

2015-07-31 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37007]

All tests passed.

- Mesos ReviewBot


On July 31, 2015, 11:27 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37007/
> ---
> 
> (Updated July 31, 2015, 11:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3016
> https://issues.apache.org/jira/browse/MESOS-3016
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, only FrameworkID and TaskID are sent to the hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b 
>   src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf 
>   src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 
>   src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 
>   src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
> 
> Diff: https://reviews.apache.org/r/37007/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>