Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-13 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148760 --- Before I land this please make the following changes: Link this

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-12 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 12, 2016, 9:29 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-12 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148566 --- Ship it! LGTM. I am not commiting this because there are

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-12 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148564 --- Fix it, then Ship it! RELEASE-NOTES.md (line 37)

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-12 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 12, 2016, 5:49 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-09 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148374 --- Ship it! LGTM

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-09 Thread Maxim Khutornenko
> On Sept. 9, 2016, 12:26 a.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java, > > line 125 > > > > > > This smells to me and seems awkward. I guess this

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-09 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 9, 2016, 6:43 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-09 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 9, 2016, 6:32 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148292 --- Overall LGTM, just some missing tests and missing error handling.

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Santhosh Kumar Shanmugham
> On Sept. 8, 2016, 11:43 a.m., Santhosh Kumar Shanmugham wrote: > > src/test/python/apache/aurora/admin/test_admin.py, line 285 > > > > > > Should this also be assert_called_once_with(None) ? > > Karthik Anantha

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Maxim Khutornenko
> On Sept. 8, 2016, 6:18 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/admin/admin.py, line 351 > > > > > > Needs clarifcation that `reconciliation_explicit_batch_size` is a > > scheduler

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Maxim Khutornenko
> On Sept. 8, 2016, 6:43 p.m., Santhosh Kumar Shanmugham wrote: > > src/test/python/apache/aurora/admin/test_admin.py, line 285 > > > > > > Should this also be assert_called_once_with(None) ? > > Karthik Anantha

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Karthik Anantha Padmanabhan
> On Sept. 8, 2016, 6:43 p.m., Santhosh Kumar Shanmugham wrote: > > src/test/python/apache/aurora/admin/test_admin.py, line 285 > > > > > > Should this also be assert_called_once_with(None) ? No - I set the option

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Karthik Anantha Padmanabhan
> On Sept. 8, 2016, 6:18 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java, > > line 123 > > > > > > Prefer using immutable `IExplicit...` wrapper

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148230 ---

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148225 --- api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 1215)

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Karthik Anantha Padmanabhan
> On Sept. 8, 2016, 2:11 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/admin/admin.py, lines 357-358 > > > > > > What if type is `foo`, etc? We should ensure the value is valid rather > > than

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-08 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 8, 2016, 6:48 a.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148128 --- Please add a line to RELEASE_NOTES.md about this change.

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148123 --- api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 1214)

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 8, 2016, 12:30 a.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Santhosh Kumar Shanmugham
> On Sept. 7, 2016, 2:45 p.m., Santhosh Kumar Shanmugham wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214 > > > > > > Did we consider combining the "explicit" and "implicit"

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Santhosh Kumar Shanmugham
> On Sept. 7, 2016, 2:45 p.m., Santhosh Kumar Shanmugham wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214 > > > > > > Did we consider combining the "explicit" and "implicit"

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Maxim Khutornenko
> On Sept. 7, 2016, 9:45 p.m., Santhosh Kumar Shanmugham wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214 > > > > > > Did we consider combining the "explicit" and "implicit"

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
> On Sept. 7, 2016, 9:45 p.m., Santhosh Kumar Shanmugham wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214 > > > > > > Did we consider combining the "explicit" and "implicit"

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
> On Sept. 7, 2016, 9:45 p.m., Santhosh Kumar Shanmugham wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214 > > > > > > Did we consider combining the "explicit" and "implicit"

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Santhosh Kumar Shanmugham
> On Sept. 7, 2016, 2:45 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/python/apache/aurora/admin/admin.py, line 342 > > > > > > 'default=0' here and the usage doc reads default as 1000. > > Karthik Anantha

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
> On Sept. 7, 2016, 9:45 p.m., Santhosh Kumar Shanmugham wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214 > > > > > > Did we consider combining the "explicit" and "implicit"

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148089 --- api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 1209

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 7, 2016, 9:08 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 7, 2016, 9:01 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Karthik Anantha Padmanabhan
> On Sept. 7, 2016, 6:14 p.m., Maxim Khutornenko wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 944-950 > > > > > > I don't think there is much value in having this struct and the related >

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-07 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review148046 --- api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 944

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

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

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-06 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review147953 --- @ReviewBot retry - Karthik Anantha Padmanabhan On Sept. 6,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-06 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review147950 --- Master (19866b5) is red with this patch.

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-06 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 6, 2016, 11:34 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-06 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 6, 2016, 11:18 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-06 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/#review147894 --- Master (5d3f945) is red with this patch.

Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation

2016-09-06 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51662/ --- (Updated Sept. 6, 2016, 6:42 p.m.) Review request for Aurora and Maxim