Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

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

Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51712/ --- (Updated Sept. 8, 2016, 5:30 p.m.) Review request for Aurora, Joshua Cohen and

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 51742: Modify the callback function passed to StatusManager

2016-09-08 Thread Kai Huang
> On Sept. 8, 2016, 9:13 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, lines 204-211 > > > > > > I see a couple of problems with this method: > > > > 1. The

Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51712/#review148288 --- Master (87ae968) is red with this patch.

Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51712/ --- (Updated Sept. 8, 2016, 4:49 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Zameer Manji
> On Sept. 8, 2016, 1:19 a.m., Stephan Erb wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 899-900 > > > > > > If this is deprecated, shouldn't you call this out in the release notes > > ad

Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Zameer Manji
> On Sept. 8, 2016, 9:37 a.m., Maxim Khutornenko wrote: > > This will most certainly break the UI. The client needs to be updated as > > well to pass both (old and new) arguments. Please, test in vagrant with > > different combinations of scheduler and client versions. I have validated that

Re: Review Request 51742: Modify the callback function passed to StatusManager

2016-09-08 Thread Kai Huang
> On Sept. 8, 2016, 9:13 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, lines 204-211 > > > > > > I see a couple of problems with this method: > > > > 1. The

Re: Review Request 51664: Document the Mesos containerizer

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

Re: Review Request 51742: Modify the callback function passed to StatusManager

2016-09-08 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51742/#review148280 --- src/test/python/apache/aurora/executor/test_status_manager.py

Re: Review Request 51664: Document the Mesos containerizer

2016-09-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51664/#review148279 --- @ReviewBot retry - Joshua Cohen On Sept. 6, 2016, 7:18 p.m.,

Re: Review Request 51746: Update e2e tests to verify running a docker image via the unified containerizer.

2016-09-08 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51746/#review148275 --- Ship it! Ship It! - Stephan Erb On Sept. 8, 2016, 10:56

Re: Review Request 51742: Modify the callback function passed to StatusManager

2016-09-08 Thread Kai Huang
> On Sept. 8, 2016, 9:13 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, lines 204-211 > > > > > > I see a couple of problems with this method: > > > > 1. The

Re: Review Request 51746: Update e2e tests to verify running a docker image via the unified containerizer.

2016-09-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51746/#review148264 --- Ship it! Master (8fca745) 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 51746: Update e2e tests to verify running a docker image via the unified containerizer.

2016-09-08 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51746/#review148262 --- Ship it! Ship It! - Zhitao Li On Sept. 8, 2016, 8:56 p.m.,

Re: Review Request 51742: Modify the callback function passed to StatusManager

2016-09-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51742/#review148258 --- src/main/python/apache/aurora/executor/aurora_executor.py (line

Review Request 51746: Update e2e tests to verify running a docker image via the unified containerizer.

2016-09-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51746/ --- Review request for Aurora, Stephan Erb and Zhitao Li. Repository: aurora

Re: Review Request 51742: Modify the callback function passed to StatusManager

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

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

Review Request 51742: Modify the callback function passed to StatusManager

2016-09-08 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51742/ --- Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.

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 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51712/#review148208 --- This will most certainly break the UI. The client needs to be

Re: Review Request 51664: Document the Mesos containerizer

2016-09-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51664/#review148185 --- Ship it! lgtm modulo the below.

Re: Review Request 51712: Extend getJobUpdateDetails to accept JobUpdateQuery

2016-09-08 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51712/#review148158 --- LGTM (+-1 the current test failure)

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,