> On April 15, 2015, 11:06 p.m., Kevin Sweeney wrote:
> > Looks like you forgot to git add build.gradle?

This was just reviewbot not applying the dependent review, which has since been 
committed.


> On April 15, 2015, 11:06 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/DriverFactoryImpl.java, 
> > line 37
> > <https://reviews.apache.org/r/33243/diff/1/?file=931107#file931107line37>
> >
> >     we typically will place a comment next to the parameter to indicate its 
> > name, but don't duplicate javadoc of APIs we're calling. In this case the 
> > comment is vague - consider annotating the parameter directly:
> >     
> >     ```java
> >     return new MesosSchedulerDriver(
> >       scheduler,
> >       frameworkInfo,
> >       master,
> >       false /* disable implicit acknowledgements */);
> >     ```
> >     
> >     instead. Also this probably escaped code review upstream but this would 
> > be a good place to use an enum instead of a boolean - consider filing an 
> > upstream ticket.

Sounds good. Enum would have been more explicit, given the effort needed for 
API / JNI changes, that ship has sailed. :)


> On April 15, 2015, 11:06 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, 
> > line 237
> > <https://reviews.apache.org/r/33243/diff/1/?file=931108#file931108line237>
> >
> >     This comment is now a lie - we are sure we've NACKed the status update.

My bad, thanks for catching this! I've left it saying that we re-throw to abort 
the driver, since SchedulerException looks to be due to only unexpected errors?


- Ben


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


On April 15, 2015, 10:47 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33243/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:47 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1228
>     https://issues.apache.org/jira/browse/AURORA-1228
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The scheduler now explicitly acknowledges updates. I left the structure as 
> is, per Bill's suggestion.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/DriverFactoryImpl.java 
> db7aa74c2dfca3d520d978d7837d1f2639e6c05c 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f40746b5712f21e551c06eeb1aa379ebdcdbc693 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> 4ef23ea6ebd2c9587a8356507fcb49a36b9de219 
> 
> Diff: https://reviews.apache.org/r/33243/diff/
> 
> 
> Testing
> -------
> 
> Ran the unit tests and the end-to-end test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to