> On April 9, 2016, 12:27 a.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, line 1032
> > <https://reviews.apache.org/r/45932/diff/1/?file=1337268#file1337268line1032>
> >
> >     Can we omit this? Why did you include it?

I'll drop this as I don't have a good reason to use it for now. The reasoning 
was to distingguish state for non-terminal tasks if necessary, although I doubt 
that.

We can always add this when we really need it.


> On April 9, 2016, 12:27 a.m., Ben Mahler wrote:
> > src/tests/oversubscription_tests.cpp, lines 230-231
> > <https://reviews.apache.org/r/45932/diff/1/?file=1337271#file1337271line230>
> >
> >     How about s/task_label/key/ s/task_label_value/value/ ? Will it fit on 
> > one line then?
> >     
> >     We generally avoid "foo" and "bar" in favor of things like "key" and 
> > "value" to make the test clearer, so please ignore the executor labels here.

I'd like to keep the ExecutorLabel test here since that is necessary. I'll 
rename foo -> exec_key and bar -> exec_value.


- Zhitao


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


On April 12, 2016, 5 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45932/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 5 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5030
>     https://issues.apache.org/jira/browse/MESOS-5030
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add stripped TaskInfo's to ResourceUsage.Executor message.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 1f0527e86e333970f7f7879bb2bcbc33f0f44bc3 
>   include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861 
>   include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f 
>   src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 
>   src/tests/oversubscription_tests.cpp 
> 23671746da2ac505d75bc2bd59114697d9161d52 
> 
> Diff: https://reviews.apache.org/r/45932/diff/
> 
> 
> Testing
> -------
> 
> Added new test to verify ResourceUsage sees task labels.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Reply via email to