----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3761/#review4920 -----------------------------------------------------------
include/mesos/executor.hpp <https://reviews.apache.org/r/3761/#comment10826> Okay, I think the right thing here is to pass both SlaveID and SlaveInfo. That way we can access the slaveInfo.hostname() and slaveInfo.public_hostname() if we need. include/mesos/executor.hpp <https://reviews.apache.org/r/3761/#comment10824> This looks like I forgot to finish this comment. I think I wanted to say: "Note, the executor pointer must outlive the driver." Can you add that please? Thanks! src/Makefile.am <https://reviews.apache.org/r/3761/#comment10825> This has been committed in another patch. src/exec/exec.cpp <https://reviews.apache.org/r/3761/#comment10827> No need for the ':'. src/exec/exec.cpp <https://reviews.apache.org/r/3761/#comment10828> s/for:/for task src/exec/exec.cpp <https://reviews.apache.org/r/3761/#comment10831> Indentation? src/exec/exec.cpp <https://reviews.apache.org/r/3761/#comment10829> Wrap line. src/exec/exec.cpp <https://reviews.apache.org/r/3761/#comment10830> This data could be binary! Let's not print it out please. src/exec/exec.cpp <https://reviews.apache.org/r/3761/#comment10832> This is more verbose than we need to be, please remove. src/python/native/proxy_executor.cpp <https://reviews.apache.org/r/3761/#comment10833> If the rest of the file was updated to spell out the variable (i.e., sidObj => slaveIdObj), then do the same here please. - Benjamin On 2012-02-05 03:34:16, Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3761/ > ----------------------------------------------------------- > > (Updated 2012-02-05 03:34:16) > > > Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman. > > > Summary > ------- > > Currently, the init() callback in Executor is not consistent with the naming > of our callbacks. Like scheduler callback, this should be called > registered(). > > In the near future, we are also going to add a new executor callback called > re-registered() as we implement slaves being restarted without killing > executors. > > As part of this, also got rid of the ExecutorArgs protobuf. > > > Diffs > ----- > > > frameworks/hadoop-0.20.2/src/contrib/mesos/src/java/org/apache/hadoop/mapred/FrameworkExecutor.java > 6744082 > include/mesos/executor.hpp 8e651e7 > include/mesos/mesos.proto b855f70 > src/Makefile.am ac0a209 > src/examples/java/TestExecutor.java 6211490 > src/examples/long_lived_executor.cpp 6ba083b > src/examples/memhog_executor.cpp 333b1d9 > src/examples/test_executor.cpp 9dd244e > src/exec/exec.cpp 48c0d0f > src/java/jni/convert.cpp 04a25dd > src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp b338847 > src/java/src/org/apache/mesos/Executor.java a05591f > src/messages/messages.proto 04dc3a0 > src/python/native/proxy_executor.hpp 3ed9e37 > src/python/native/proxy_executor.cpp 30cf089 > src/python/src/mesos.py 026a611 > src/scaling/nested_exec.py 5c4edf1 > src/sched/sched.cpp 21d670f > src/slave/slave.cpp 9b33a5a > src/tests/fault_tolerance_tests.cpp 7be3d6b > src/tests/master_tests.cpp a411ba5 > src/tests/utils.hpp 660aee9 > > Diff: https://reviews.apache.org/r/3761/diff > > > Testing > ------- > > ./mesos-tests --gtest_filter = -"*Python*" > > All the tests, except the SamplePythonFramework, pass. > > Looking at earlier Jiras, looks like there is a fix for python tests, that > needs to be pushed to the trunk? I'm not sure if that fixes this test fail > yet. > > > Thanks, > > Vinod > >
