Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51763/#review149182 --- Ship it! Ship It! - Stephan Erb On Sept. 15, 2016, 1:12

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/#review149179 ---

Re: Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51973/ --- (Updated Sept. 16, 2016, 1:49 p.m.) Review request for Aurora, Joshua Cohen

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Joshua Cohen
> On Sept. 15, 2016, 10:58 a.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/BatchWorker.java, lines 166-167 > > > > > > super nitpicky: mind swapping the order of these args to keep inline >

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/#review149290 --- @ReviewBot retry - Maxim Khutornenko On Sept. 16, 2016, 9:53

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/#review149294 --- Ship it! Master (a87ad41) is green with this patch.

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread Maxim Khutornenko
> On Sept. 15, 2016, 6:25 p.m., Zameer Manji wrote: > > config/findbugs/excludeFilter.xml, line 123 > > > > > > If I'm reading the code correctly we need this because we have a > > `CompletableFuture` and to give

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51763/#review149279 --- Ship it! Master (f1e09a9) is green with this patch.

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread John Sirois
> On Sept. 15, 2016, 12:25 p.m., Zameer Manji wrote: > > config/findbugs/excludeFilter.xml, line 123 > > > > > > If I'm reading the code correctly we need this because we have a > > `CompletableFuture` and to give

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review149263 --- Ship it! Master (783baae) is green with this patch.

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51763/ --- (Updated Sept. 16, 2016, 9:04 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/ --- (Updated Sept. 16, 2016, 7:53 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Maxim Khutornenko
> On Sept. 16, 2016, 6:18 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/BatchWorker.java, line 213 > > > > > > I think this is a misuse of `CompletalbleFuture` I discovered this as I > >

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Maxim Khutornenko
> On Sept. 15, 2016, 10:58 a.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/BatchWorker.java, lines 166-167 > > > > > > super nitpicky: mind swapping the order of these args to keep inline >

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review149256 --- Ship it! Ship It! - Zameer Manji On Sept. 16, 2016, 1:03

Re: Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51973/#review149266 --- Not that it changes much but since it's technically a schema

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/#review149288 --- Master (496397a) is red with this patch.

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Maxim Khutornenko
> On Sept. 16, 2016, 9:08 a.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java, line > > 197 > > > > > > Side show: Isn't that `if` unnecessary here and we can adjust

Re: Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51973/#review149272 --- Ship it! Master (783baae) is green with this patch.

Re: Review Request 51874: Change framework_name default value from 'TwitterScheduler' to 'Aurora'

2016-09-16 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/ --- (Updated Sept. 16, 2016, 2:50 p.m.) Review request for Aurora, Joshua Cohen

Re: Review Request 51765: Batching writes - Part 3 (of 3): Converting TaskScheduler to use BatchWorker.

2016-09-16 Thread Maxim Khutornenko
> On Sept. 16, 2016, 6:28 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/scheduling/SchedulingModule.java, > > line 91 > > > > > > Curious, arbitrary or derrived? Arbitrary derived :) There

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/ --- (Updated Sept. 16, 2016, 8:33 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51973/#review149273 --- Ship it! Ship It! - Joshua Cohen On Sept. 16, 2016, 8:49

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51763/#review149275 --- Ship it! Ship It! - Joshua Cohen On Sept. 14, 2016, 11:12

Re: Review Request 51874: Change framework_name default value from 'TwitterScheduler' to 'Aurora'

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/#review149286 --- Ship it! Master (496397a) is green with this patch.

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread John Sirois
> On Sept. 15, 2016, 12:25 p.m., Zameer Manji wrote: > > config/findbugs/excludeFilter.xml, line 123 > > > > > > If I'm reading the code correctly we need this because we have a > > `CompletableFuture` and to give

Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51973/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Repository:

Re: Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Renan DelValle
> On Sept. 16, 2016, 1:42 p.m., Maxim Khutornenko wrote: > > Not that it changes much but since it's technically a schema change a > > release note would be great. Doh! Can't believe I forgot. Coming right up. - Renan --- This is an

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51980/#review149311 --- Ship it! Master (a87ad41) is green with this patch.

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-16 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51980/ --- (Updated Sept. 17, 2016, 1:40 a.m.) Review request for Aurora, Maxim

Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-16 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51980/ --- Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs:

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/#review149260 --- Master (783baae) is red with this patch.

Re: Review Request 51763: Batching writes - Part 2 (of 3): Converting cron jobs to use BatchWorker.

2016-09-16 Thread Maxim Khutornenko
> On Sept. 15, 2016, 6:25 p.m., Zameer Manji wrote: > > config/findbugs/excludeFilter.xml, line 123 > > > > > > If I'm reading the code correctly we need this because we have a > > `CompletableFuture` and to give

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/ --- (Updated Sept. 16, 2016, 9:53 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51929: Scheduling multiple tasks per round.

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51929/ --- (Updated Sept. 16, 2016, 9:53 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51973: Fix for AURORA-1739, enables golang thrift bindings to create jobs

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51973/#review149271 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 16, 2016,

Re: Review Request 51765: Batching writes - Part 3 (of 3): Converting TaskScheduler to use BatchWorker.

2016-09-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51765/#review149277 --- Ship it! Ship It! - Joshua Cohen On Sept. 14, 2016, 11:18

Re: Review Request 51874: Change framework_name default value from 'TwitterScheduler' to 'Aurora'

2016-09-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/#review149216 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 15, 2016,

Re: Review Request 51759: Batching writes - Part 1 (of 3): Introducing BatchWorker and task event batching.

2016-09-16 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review149227 --- src/main/java/org/apache/aurora/scheduler/BatchWorker.java (line

Re: Review Request 51765: Batching writes - Part 3 (of 3): Converting TaskScheduler to use BatchWorker.

2016-09-16 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51765/#review149229 --- Ship it! LGTM.