You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Isabel Jimenez <co...@isabeljimenez.com> on 2015/10/06 01:51:54 UTC

Re: Review Request 38874: Refactored executor struct in Agent for the Executor HTTP API

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



src/slave/slave.hpp (line 42)
<https://reviews.apache.org/r/38874/#comment158988>

    we don't need this



src/slave/slave.cpp (line 4054)
<https://reviews.apache.org/r/38874/#comment158989>

    `executor->pid.isSome() && executor->pid.get() != UPID()` ?



src/slave/slave.cpp (line 4069)
<https://reviews.apache.org/r/38874/#comment158990>

    same as above



src/slave/slave.cpp (line 5340)
<https://reviews.apache.org/r/38874/#comment158992>

    same as above


- Isabel Jimenez


On Sept. 30, 2015, 3:39 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38874/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2015, 3:39 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3480
>     https://issues.apache.org/jira/browse/MESOS-3480
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change refactors the Executor struct on Agent and adds support for Executors to connect via the `api/v1/executor` endpoint on Agent. This is similar to the change done in Master for the Scheduler HTTP API.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38874/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 38874: Refactored executor struct in Agent for the Executor HTTP API

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Oct. 5, 2015, 11:51 p.m., Isabel Jimenez wrote:
> > src/slave/slave.hpp, line 42
> > <https://reviews.apache.org/r/38874/diff/1/?file=1087513#file1087513line42>
> >
> >     we don't need this

We need this include as the encoder needs it. We can get away with this but that would entail converting the old message to unversioned protobuf and then evolving it again ( 2 conversions ) instead of 1 that is the case now.

The same logic exists in the master too for the same reason. We can revisit this later.


> On Oct. 5, 2015, 11:51 p.m., Isabel Jimenez wrote:
> > src/slave/slave.cpp, line 4054
> > <https://reviews.apache.org/r/38874/diff/1/?file=1087514#file1087514line4054>
> >
> >     `executor->pid.isSome() && executor->pid.get() != UPID()` ?

They both mean the same in this context.

https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/pid.hpp#L69


- Anand


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


On Oct. 6, 2015, 2:22 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38874/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 2:22 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3480
>     https://issues.apache.org/jira/browse/MESOS-3480
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change refactors the Executor struct on Agent and adds support for Executors to connect via the `api/v1/executor` endpoint on Agent. This is similar to the change done in Master for the Scheduler HTTP API.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38874/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>