> On Oct. 24, 2014, 11:08 a.m., Vinod Kone wrote:
> > include/mesos/mesos.proto, lines 624-629
> > <https://reviews.apache.org/r/26382/diff/7/?file=727940#file727940line624>
> >
> >     Pull this one and Reason inside TaskStatus.
> >     
> >     Also kill the SOURCE_ and REASON_ prefixes.

and not enum TaskState? I was following that model.

i'd like to keep the prefixes as it makes it easy to be sure in the callsite 
that you have the right enum. Autocomplete then works nicely too.


> On Oct. 24, 2014, 11:08 a.m., Vinod Kone wrote:
> > include/mesos/mesos.proto, line 636
> > <https://reviews.apache.org/r/26382/diff/7/?file=727940#file727940line636>
> >
> >     I think this will become a crutch. We should know the reasons for why a 
> > task is lost. i.e., kill this.

it still might be 'unknown' in cases where the caller doesn't set the reason. 
is that acceptable?


> On Oct. 24, 2014, 11:08 a.m., Vinod Kone wrote:
> > include/mesos/mesos.proto, line 654
> > <https://reviews.apache.org/r/26382/diff/7/?file=727940#file727940line654>
> >
> >     s/NOT_ENOUGH_RESOURCES/INVALID_RESOURCES/
> >     
> >     also, how is this different from FAILED_VALIDATION?

there's a check in master that we have enough resources. maybe this is dead 
code duplicating a task validation step?


> On Oct. 24, 2014, 11:08 a.m., Vinod Kone wrote:
> > src/common/http.cpp, line 103
> > <https://reviews.apache.org/r/26382/diff/7/?file=727941#file727941line103>
> >
> >     what happens if this is not set?

nothing - i thought it was a useful thing to export in the JSON model though.


> On Oct. 24, 2014, 11:08 a.m., Vinod Kone wrote:
> > src/examples/java/TestFramework.java, lines 135-136
> > <https://reviews.apache.org/r/26382/diff/7/?file=727947#file727947line135>
> >
> >     looks like you missed "from source" in other example frameworks above.

the balloon framework has it.. i'll double check i put it everywhere else.


> On Oct. 24, 2014, 11:08 a.m., Vinod Kone wrote:
> > src/master/master.cpp, line 2301
> > <https://reviews.apache.org/r/26382/diff/7/?file=727958#file727958line2301>
> >
> >     why is this TASK_ERROR instead of TASK_LOST? A relaunch would likely 
> > succeed here right?

I thought if the offer id is invalid it likely would be again next time. Is 
that not the case?


- Dominic


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


On Oct. 27, 2014, 10:18 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26382/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2014, 10:18 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Bill Farner.
> 
> 
> Bugs: MESOS-1830 and MESOS-343
>     https://issues.apache.org/jira/browse/MESOS-1830
>     https://issues.apache.org/jira/browse/MESOS-343
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added source and reason, enabled TASK_ERROR, and made the changes necessary 
> throughout the codebase.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
>   src/common/protobuf_utils.hpp 212d5124b9a4cc58e61719fa7f07a61cd166e834 
>   src/common/protobuf_utils.cpp a9b65e328c4c62bff7fbf5633dda25d742d79019 
>   src/examples/balloon_framework.cpp b05d5679fe2915142907af0b2dc00c6cd76eb9c1 
>   src/examples/java/TestFramework.java 
> bc593d0abfacb00690b1492b2b82c970f4e4de6d 
>   src/examples/low_level_scheduler_libprocess.cpp 
> 7ef5ea78ade4ed856b97009fdfe31281f0a55c17 
>   src/examples/low_level_scheduler_pthread.cpp 
> 6e233a10117a1c7aa669806b5b430e746e227ee5 
>   src/examples/no_executor_framework.cpp 
> f98a0735b9f287e7f1bf98af6c2e9a47ca6a77b2 
>   src/examples/test_framework.cpp 187a611ebfe35cb13ee48aa5eca934cf55f34dea 
>   src/master/master.cpp 9ebdc357fe679ed5bd54e0f4b9091ab9bcc2b00f 
>   src/scaling/nested_exec.py 17e61706c75e6e849b0c40ae5232d8dc60804c55 
>   src/sched/sched.cpp 0fb8c7bda75545389f8024489b3c76ae115111f4 
>   src/slave/slave.cpp 5e7c107aa8f36aff954fa6a46c431a20c313ebc2 
>   src/tests/common/http_tests.cpp 912653b26615c86cc0204d80f58e6046c4b91a98 
>   src/tests/fault_tolerance_tests.cpp 
> a18a41a3e34ff112e04e693447d757403e5013bd 
>   src/tests/master_authorization_tests.cpp 
> 652e80d0d4567b225c6ffb326725ddfde06f7fd3 
>   src/tests/master_tests.cpp f60e3761e5a558d379c56513937d2f59e898a490 
>   src/tests/reconciliation_tests.cpp 4ba53940951584d9baa2c7aa4e124814471206bc 
>   src/tests/resource_offers_tests.cpp 
> fe66432b1bf75ee25feb73c4bb353e4d4e5b503f 
> 
> Diff: https://reviews.apache.org/r/26382/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Reply via email to