> On July 15, 2015, 9:38 p.m., Ben Mahler wrote:
> > src/tests/scheduler_tests.cpp, line 143
> > <https://reviews.apache.org/r/36518/diff/1/?file=1012851#file1012851line143>
> >
> >     Hm.. 'force' doesn't make sense for SUBSCRIBE without a framework id.
> >     
> >     Seems like either we:
> >     
> >     (1) Make 'force' optional, and document that it is only relevant when 
> > the framework id is set (re-subscription).
> >     
> >     (2) Remove 'force' from SUBSCRIBE, add a RESUBSCRIBE with 'force'? =/

I'll make 'force' optional in a dependent different review.


> On July 15, 2015, 9:38 p.m., Ben Mahler wrote:
> > src/tests/scheduler_tests.cpp, line 163
> > <https://reviews.apache.org/r/36518/diff/1/?file=1012851#file1012851line163>
> >
> >     why the newline here but not above?

looked dense. will remove.


> On July 15, 2015, 9:38 p.m., Ben Mahler wrote:
> > src/tests/scheduler_tests.cpp, lines 110-112
> > <https://reviews.apache.org/r/36518/diff/1/?file=1012851#file1012851line110>
> >
> >     Since there's no RESUBSCRIBE, shall we simply call this test 
> > 'Subscribe' (I noticed there is no Subscribe test currently) and test the 
> > full semantics within it? Looks like a test of the 'Subscribe' call to me.

done.


> On July 15, 2015, 9:38 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1747
> > <https://reviews.apache.org/r/36518/diff/1/?file=1012850#file1012850line1747>
> >
> >     Seems like we might want to keep the condition consistent across all of 
> > these checks (i.e. has_id && id != ""), up to you.
> >     
> >     At least, would be nice to add an != operator for FrameworkID vs string.

updated the condition. i'll add a TODO to add != operator and do a sweep.


- Vinod


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


On July 15, 2015, 6:40 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36518/
> -----------------------------------------------------------
> 
> (Updated July 15, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3055
>     https://issues.apache.org/jira/browse/MESOS-3055
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the process of fixing this, added an additional check in 
> Master::registerFramework() that should've been there in the first place. 
> Similar check exists in Master::reregisterFramework().
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
>   src/tests/scheduler_tests.cpp 946fa8245d8ab35e04bad642d69114caf0ccf6a9 
> 
> Diff: https://reviews.apache.org/r/36518/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to