Re: Review Request 21349: Starting SLA calculations on SchedulerActive event.

2014-05-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21349/ --- (Updated May 15, 2014, 12:26 a.m.) Review request for Aurora and Bill Farner.

Re: Review Request 21247: Add config grouping visualisation to job page

2014-05-16 Thread David McLaughlin
On May 10, 2014, 2:14 a.m., Suman Karumuri wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 385 https://reviews.apache.org/r/21247/diff/1/?file=577123#file577123line385 please only pass the required fields into the fields into the function.

Re: Review Request 21250: Removed cron jobs table from role and env page. Added cron job summary to job page.

2014-05-16 Thread Suman Karumuri
On May 13, 2014, 12:26 a.m., David McLaughlin wrote: File Attachment: job summary of cron job - Screen Shot 2014-05-11 at 1.37.54 PM.png https://reviews.apache.org/r/21250/#fcomment18 There is no distinction between label/value nor whitespace or borders to separate different

Re: Review Request 21297: Adding UpdateConfig value checks.

2014-05-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21297/ --- (Updated May 15, 2014, 12:59 a.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 21407: Add JSHint to our build

2014-05-16 Thread David McLaughlin
On May 15, 2014, 5:06 p.m., Suman Karumuri wrote: build.gradle, line 246 https://reviews.apache.org/r/21407/diff/2/?file=581989#file581989line246 Should we be adding generated thrift sources here? 'ACTIVE_STATES' is not defined.

Re: Review Request 21407: Add JSHint to our build

2014-05-16 Thread Suman Karumuri
On May 15, 2014, 5:06 p.m., Suman Karumuri wrote: build.gradle, line 246 https://reviews.apache.org/r/21407/diff/2/?file=581989#file581989line246 Should we be adding generated thrift sources here? 'ACTIVE_STATES' is not defined.

Re: Review Request 21297: Adding UpdateConfig value checks.

2014-05-16 Thread Maxim Khutornenko
On May 15, 2014, 12:11 a.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/config.py, line 161 https://reviews.apache.org/r/21297/diff/5/?file=582112#file582112line161 Also worth noting is that this warning is meaningless if the user doesn't have health checking

Re: Review Request 21407: Add JSHint to our build

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21407/#review43182 --- The code is pushed to master. Please close out this review and the

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

2014-05-16 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18979/#review43117 --- Ship it! awesome!!! - Joe Smith On May 12, 2014, 5:15 a.m.,

Re: Review Request 21250: Removed cron jobs table from role and env page. Added cron job summary to job page.

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21250/#review43160 ---

Re: Review Request 21497: Add CORS support to thrift end points.

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21497/ --- (Updated May 16, 2014, 12:16 a.m.) Review request for Aurora, David McLaughlin

Re: Review Request 21297: Adding UpdateConfig value checks.

2014-05-16 Thread Maxim Khutornenko
On May 15, 2014, 12:27 a.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/config.py, line 132 https://reviews.apache.org/r/21297/diff/5/?file=582112#file582112line132 I find health notifications to be delivered to be vague. I prefer the wording In order for the

Re: Review Request 21502: Fix thrift tests.

2014-05-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21502/#review43183 --- Ship it! Ship It! - Maxim Khutornenko On May 15, 2014, 10:56

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

2014-05-16 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18979/ --- (Updated May 15, 2014, 12:02 p.m.) Review request for Aurora and Joe Smith.

Re: Review Request 21402: Add python checkstyle hooks.

2014-05-16 Thread Dan Norris
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21402/#review43078 --- build-support/python/checkstyle-check

Re: Review Request 21247: Add config grouping visualisation to job page

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21247/ --- (Updated May 15, 2014, 9:47 p.m.) Review request for Aurora, Suman Karumuri

Re: Review Request 21459: Database-backed implementation of QuotaStore.

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21459/#review43170 --- Ship it!

Re: Review Request 21132: Initial attempt at h2/DB storage implementation (LockStore only)

2014-05-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21132/#review43185 ---

Re: Review Request 21402: Add python checkstyle hooks.

2014-05-16 Thread Brian Wickman
On May 15, 2014, 2:01 a.m., Dan Norris wrote: build-support/python/checkstyle-check, line 23 https://reviews.apache.org/r/21402/diff/2/?file=580886#file580886line23 Could you consolidate checkstyle and checkstyle check by sourcing the venv and calling deactivate once you're done?

Review Request 21273: Add a config noun with a list verb to list jobs defined in a config file.

2014-05-16 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21273/ --- Review request for Aurora, David McLaughlin and Suman Karumuri. Bugs:

Re: Review Request 21247: Add config grouping visualisation to job page

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21247/#review43180 --- This diff is merged. Please close out this review and the

Re: Review Request 19796: AURORA-145:Test dependencies leak into distribution

2014-05-16 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19796/#review43173 --- Ship it! I'm +1 if this works on master now. - Kevin Sweeney On

Re: Review Request 18537: AURORA-227: Aurora build should check for the Python version

2014-05-16 Thread Dan Norris
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18537/#review43083 --- kevints: Is there anything still blocking this that you see? It

Re: Review Request 18537: AURORA-227: Aurora build should check for the Python version

2014-05-16 Thread Dan Norris
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18537/ --- (Updated May 15, 2014, 2:47 a.m.) Review request for Aurora, Kevin Sweeney and

Re: Review Request 21273: Add a config noun with a list verb to list jobs defined in a config file.

2014-05-16 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21273/#review43208 --- src/main/python/apache/aurora/client/cli/config.py

Re: Review Request 21523: Make JS compliant with JSHint rules

2014-05-16 Thread David McLaughlin
On May 16, 2014, 5:05 p.m., Mark Chu-Carroll wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 17 https://reviews.apache.org/r/21523/diff/1/?file=582876#file582876line17 Why? David McLaughlin wrote: Good question. JavaScript

Re: Review Request 21250: Removed cron jobs table from role and env page. Added cron job summary to job page.

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21250/#review43164 --- Ship it! Ship It! - David McLaughlin On May 15, 2014, 9:07

Re: Review Request 21523: Make JS compliant with JSHint rules

2014-05-16 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21523/#review43218 --- Ship it! Ship It!

Review Request 21523: Make JS compliant with JSHint rules

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21523/ --- Review request for Aurora, Suman Karumuri and Mark Chu-Carroll. Repository:

Re: Review Request 21497: Add CORS support to thrift end points.

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21497/ --- (Updated May 15, 2014, 7:28 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 21497: Add CORS support to thrift end points.

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21497/#review43155 --- src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java

Re: Review Request 21132: Initial attempt at h2/DB storage implementation (LockStore only)

2014-05-16 Thread David McLaughlin
On May 16, 2014, 4 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java, line 43 https://reviews.apache.org/r/21132/diff/3/?file=580759#file580759line43 Breaking the abstraction by having a JobKeyMapper here is quite unfortunate.

Re: Review Request 21250: Removed cron jobs table from role and env page. Added cron job summary to job page.

2014-05-16 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21250/#review43156 --- Ship it! Display and java code LGTM. I'll let David field the JS

Re: Review Request 21523: Make JS compliant with JSHint rules

2014-05-16 Thread David McLaughlin
On May 16, 2014, 5:05 p.m., Mark Chu-Carroll wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 17 https://reviews.apache.org/r/21523/diff/1/?file=582876#file582876line17 Why? Good question. JavaScript in the browser doesn't have modules or

Re: Review Request 21440: Implementing parallel updater

2014-05-16 Thread Maxim Khutornenko
On May 14, 2014, 10:57 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/updater.py, line 237 https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line237 it surprises me that this is necessary. will signal.signal() even work outside of the

Re: Review Request 21497: Add CORS support to thrift end points.

2014-05-16 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21497/#review43234 --- src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java

Re: Review Request 21250: Removed cron jobs table from role and env page. Added cron job summary to job page.

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21250/ --- (Updated May 15, 2014, 9:07 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 21250: Removed cron jobs table from role and env page. Added cron job summary to job page.

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21250/ --- (Updated May 15, 2014, 12:46 a.m.) Review request for Aurora, David McLaughlin

Review Request 21294: Only serve thrift over HTTP.

2014-05-16 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21294/ --- Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Maxim

Re: Review Request 20407: AURORA-323:Add ability to merge in github pull requests similar to rbt

2014-05-16 Thread Jake Farrell
On April 25, 2014, 4:06 a.m., Bill Farner wrote: I have some reservations with this, but they're not well-formed. I like the idea of giving committers a chance to chime in on reviews, and we seem to be building process around reviewboard for that. We also have plans to build test

Review Request 21459: Database-backed implementation of QuotaStore.

2014-05-16 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21459/ --- Review request for Aurora, David McLaughlin and Kevin Sweeney. Bugs:

Re: Review Request 21426: Added a nav bar with Aurora logo.

2014-05-16 Thread Chris Lambert
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21426/#review43018 --- Are you using the correct logo (from AURORA-229)? It's hard to

Re: Review Request 21455: Opt-in for code quality checks to speed up development iteration.

2014-05-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21455/#review43177 --- build.gradle https://reviews.apache.org/r/21455/#comment77247

Re: Review Request 21273: Add a config noun with a list verb to list jobs defined in a config file.

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21273/#review43138 --- Ship it! src/main/python/apache/aurora/client/cli/config.py

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

2014-05-16 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18979/ --- (Updated May 15, 2014, 12:08 p.m.) Review request for Aurora and Joe Smith.

Review Request 21502: Fix thrift tests.

2014-05-16 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21502/ --- Review request for Aurora, Maxim Khutornenko and Brian Wickman. Repository:

Re: Review Request 19788: Add a clientv2 version of the e2e test.

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19788/#review42417 --- src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh

Re: Review Request 21497: Add CORS support to thrift end points.

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21497/#review43192 --- Ship it! Ship It! - David McLaughlin On May 16, 2014, 12:16

Re: Review Request 21440: Implementing parallel updater

2014-05-16 Thread Maxim Khutornenko
On May 14, 2014, 7:20 p.m., Mark Chu-Carroll wrote: src/test/python/apache/aurora/client/api/test_updater.py, line 126 https://reviews.apache.org/r/21440/diff/1/?file=581731#file581731line126 Doesn't this mean that the test will be running with just one thread? That will

Re: Review Request 21386: Add support for custom project to list-missing-shipits

2014-05-16 Thread Jake Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21386/#review43080 --- please create a jira issue for this and then +1 - Jake Farrell

Re: Review Request 21497: Add CORS support to thrift end points.

2014-05-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21497/ --- (Updated May 16, 2014, 12:16 a.m.) Review request for Aurora, David

Re: Review Request 21523: Make JS compliant with JSHint rules

2014-05-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21523/ --- (Updated May 16, 2014, 1:30 a.m.) Review request for Aurora, Suman Karumuri