Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Giulio Eulisse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/ --- Review request for Aurora. Repository: aurora Description --- This allow

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/#review148913 --- Master (5069f93) is red with this patch. ./build-support/jenkins

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Giulio Eulisse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/ --- (Updated Sept. 14, 2016, 4:03 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/#review148914 --- Master (5069f93) is red with this patch. ./build-support/jenkins

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Giulio Eulisse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/ --- (Updated Sept. 14, 2016, 4:17 p.m.) Review request for Aurora, Joshua Cohen and

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

2016-09-14 Thread Joshua Cohen
> On Sept. 14, 2016, 12:11 a.m., Zameer Manji wrote: > > I support this change as a developer. > > > > As an operator I am scared. > > > > What happens to an existing cluster if we don't set `framework_name`? Will > > it register another frameowork_id? (bad) or will it fail to register? > > (

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/#review148920 --- Master (5069f93) is red with this patch. ./build-support/jenkins

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

2016-09-14 Thread Maxim Khutornenko
> On Sept. 14, 2016, 1:29 a.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/BatchWorker.java, line 110 > > > > > > The documentation implies we are returning a boolean but we are > > returning

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

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

Re: Review Request 51876: @ReviewBot retry Modify executor state transition logic to rely on health checks (if enabled)

2016-09-14 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review148925 --- src/main/python/apache/aurora/executor/common/health_checker.py (

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

2016-09-14 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review148918 --- Fix it, then Ship it! LGTM. A couple of smaller points below,

Re: Review Request 51876: @ReviewBot retry Modify executor state transition logic to rely on health checks (if enabled)

2016-09-14 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review148927 --- src/main/python/apache/aurora/executor/common/health_checker.py (

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

2016-09-14 Thread Stephan Erb
> On Sept. 14, 2016, 6:56 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/scheduling/TaskThrottler.java, > > lines 76-90 > > > > > > This code is exposing a slight design smell in my eye: We ar

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

2016-09-14 Thread Maxim Khutornenko
> On Sept. 14, 2016, 4:56 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/scheduling/TaskThrottler.java, > > lines 76-90 > > > > > > This code is exposing a slight design smell in my eye: We ar

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

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review148931 --- Ship it! Master (5069f93) is green with this patch. ./build-s

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

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

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

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review148936 --- Master (5069f93) is red with this patch. ./build-support/jenkins

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

2016-09-14 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51763/#review148933 --- src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJ

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

2016-09-14 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51765/#review148941 --- Ship it! Seems to be a straight forward usage of the BatchWorke

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Giulio Eulisse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/ --- (Updated Sept. 14, 2016, 6:19 p.m.) Review request for Aurora, Joshua Cohen and

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

2016-09-14 Thread Zameer Manji
> On Sept. 13, 2016, 6:29 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/BatchWorker.java, line 110 > > > > > > The documentation implies we are returning a boolean but we are > > returning

Re: Review Request 51893: Allow cookie based authentication

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51893/#review148949 --- Master (5069f93) is red with this patch. ./build-support/jenkins

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

2016-09-14 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review148950 --- Fix it, then Ship it! This patch LGTM. Just a single concern he

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

2016-09-14 Thread Santhosh Kumar Shanmugham
> On Sept. 13, 2016, 5:11 p.m., Zameer Manji wrote: > > I support this change as a developer. > > > > As an operator I am scared. > > > > What happens to an existing cluster if we don't set `framework_name`? Will > > it register another frameowork_id? (bad) or will it fail to register? > > (b

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

2016-09-14 Thread Santhosh Kumar Shanmugham
> On Sept. 13, 2016, 5:11 p.m., Zameer Manji wrote: > > I support this change as a developer. > > > > As an operator I am scared. > > > > What happens to an existing cluster if we don't set `framework_name`? Will > > it register another frameowork_id? (bad) or will it fail to register? > > (b

Review Request 51899: Ensure shell health checkers running for tasks running under an isolated fileystem are run within that filesystem.

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

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

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

Re: Review Request 51899: Ensure shell health checkers running for tasks running under an isolated fileystem are run within that filesystem.

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51899/#review148970 --- Ship it! Master (5069f93) is green with this patch. ./build-s

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

2016-09-14 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/#review148971 --- Ship it! Thanks for the detailed testing! LGTM. It seems like w

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

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/#review148972 --- Ship it! Master (5069f93) is green with this patch. ./build-s

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

2016-09-14 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/#review148973 --- Ship it! Ship It! - Joshua Cohen On Sept. 14, 2016, 8:58 p.m

Review Request 51900: Add reconciliation_explicit_batch_size scheduler option to the documentation

2016-09-14 Thread Karthik Anantha Padmanabhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51900/ --- Review request for Aurora and Joshua Cohen. Repository: aurora Description --

Re: Review Request 51900: Add reconciliation_explicit_batch_size scheduler option to the documentation

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

Re: Review Request 51900: Add reconciliation_explicit_batch_size scheduler option to the documentation

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51900/#review148978 --- Ship it! Master (5069f93) is green with this patch. ./build-s

Re: Review Request 51876: @ReviewBot retry Modify executor state transition logic to rely on health checks (if enabled)

2016-09-14 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review148937 --- src/main/python/apache/aurora/executor/aurora_executor.py (line 1

Re: Review Request 51876: @ReviewBot retry Modify executor state transition logic to rely on health checks (if enabled)

2016-09-14 Thread Kai Huang
> On Sept. 14, 2016, 10:03 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/common/status_checker.py, line 104 > > > > > > Not obvious to me: what's the purpose of this condition here? Oh, I

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

2016-09-14 Thread Maxim Khutornenko
> On Sept. 14, 2016, 6:36 p.m., Zameer Manji wrote: > > This patch LGTM. Just a single concern here and some questions. I will be > > moving on to the subsequent patches shortly. > > Please do not commit this until all parts are shipped. Also, please don't > > forget to rebase/update subsequent

Re: Review Request 51876: @ReviewBot retry Modify executor state transition logic to rely on health checks (if enabled)

2016-09-14 Thread Kai Huang
> On Sept. 14, 2016, 10:03 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, lines 40-43 > > > > > > Do we really need this enum? A task that is in the middle of the

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

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

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

2016-09-14 Thread Zameer Manji
> On Sept. 14, 2016, 11:36 a.m., Zameer Manji wrote: > > This patch LGTM. Just a single concern here and some questions. I will be > > moving on to the subsequent patches shortly. > > Please do not commit this until all parts are shipped. Also, please don't > > forget to rebase/update subsequen

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

2016-09-14 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51874/#review148988 --- src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriver

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

2016-09-14 Thread Maxim Khutornenko
> On Sept. 14, 2016, 6:36 p.m., Zameer Manji wrote: > > This patch LGTM. Just a single concern here and some questions. I will be > > moving on to the subsequent patches shortly. > > Please do not commit this until all parts are shipped. Also, please don't > > forget to rebase/update subsequent

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

2016-09-14 Thread Zameer Manji
> On Sept. 14, 2016, 11:36 a.m., Zameer Manji wrote: > > This patch LGTM. Just a single concern here and some questions. I will be > > moving on to the subsequent patches shortly. > > Please do not commit this until all parts are shipped. Also, please don't > > forget to rebase/update subsequen

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

2016-09-14 Thread Maxim Khutornenko
> On Sept. 14, 2016, 5:40 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java, > > lines 164-172 > > > > > > I probably don't completely understand the logic, so stu

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

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

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

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51759/#review148994 --- Ship it! Master (5069f93) is green with this patch. ./build-s

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

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

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

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

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

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

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

2016-09-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51765/#review148998 --- Ship it! Master (5069f93) is green with this patch. ./build-s

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

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

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

2016-09-14 Thread Santhosh Kumar Shanmugham
> On Sept. 14, 2016, 3:48 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java, > > line 82 > > > > > > Did you try to rollback to pre 0.15 scheduler