Re: Review Request 26478: Add a flag to deduplicate storage snapshots

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26478/#review55943
---

Ship it!


Ship It!

- Zameer Manji


On Oct. 8, 2014, 7:39 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26478/
> ---
> 
> (Updated Oct. 8, 2014, 7:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-722
> https://issues.apache.org/jira/browse/AURORA-722
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a new format for deduplicated storage snapshots. Microbenchmarks show a 
> 10x deduplication ratio on Twitter's production snapshots.
> 
> This format is backwards-incompatible, so this patch introduces a flag to 
> control its use (defaulting off).
> 
> This only changes the format used to write to the replicated log (where time 
> is of the essence since all writes are done holding the global storage lock) 
> - the format of backups written to disk is unchanged, as backups don't hold 
> the lock.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 65e986eaa2c4193431ca048425a1ed3ab60f5882 
>   src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 
> 7239a6a5eb5479e395e16423c83fdf80a77e5a83 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
> 4b50e2069407dc263b4fc93f1827d3a8836253bf 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> f806297d1d0700155c976743f936b2b8a3a390fb 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 769348e6b8a5c701734afff391b1c77de35222c6 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 
> 22db80eaf34fe736fa5a3a9289836c9ac9e59906 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
> e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 5350ec945fbe028ee4641683815a068ce00b5efc 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
> 39729b374fe4e383f9b5ada7d016923766df9af7 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7a8c3b882633376a1bf6a78616d55aaa7401d13f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/26478/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26478: Add a flag to deduplicate storage snapshots

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26478/
---

(Updated Oct. 8, 2014, 7:39 p.m.)


Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.


Changes
---

Reduce visibility of some classes


Bugs: AURORA-722
https://issues.apache.org/jira/browse/AURORA-722


Repository: aurora


Description
---

Add a new format for deduplicated storage snapshots. Microbenchmarks show a 10x 
deduplication ratio on Twitter's production snapshots.

This format is backwards-incompatible, so this patch introduces a flag to 
control its use (defaulting off).

This only changes the format used to write to the replicated log (where time is 
of the essence since all writes are done holding the global storage lock) - the 
format of backups written to disk is unchanged, as backups don't hold the lock.


Diffs (updated)
-

  build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
65e986eaa2c4193431ca048425a1ed3ab60f5882 
  src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 
7239a6a5eb5479e395e16423c83fdf80a77e5a83 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
4b50e2069407dc263b4fc93f1827d3a8836253bf 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
f806297d1d0700155c976743f936b2b8a3a390fb 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
769348e6b8a5c701734afff391b1c77de35222c6 
  
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 
22db80eaf34fe736fa5a3a9289836c9ac9e59906 
  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
5350ec945fbe028ee4641683815a068ce00b5efc 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
39729b374fe4e383f9b5ada7d016923766df9af7 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
7a8c3b882633376a1bf6a78616d55aaa7401d13f 
  
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/26478/diff/


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Review Request 26478: Add a flag to deduplicate storage snapshots

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26478/
---

Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.


Bugs: AURORA-722
https://issues.apache.org/jira/browse/AURORA-722


Repository: aurora


Description
---

Add a new format for deduplicated storage snapshots. Microbenchmarks show a 10x 
deduplication ratio on Twitter's production snapshots.

This format is backwards-incompatible, so this patch introduces a flag to 
control its use (defaulting off).

This only changes the format used to write to the replicated log (where time is 
of the essence since all writes are done holding the global storage lock) - the 
format of backups written to disk is unchanged, as backups don't hold the lock.


Diffs
-

  build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
65e986eaa2c4193431ca048425a1ed3ab60f5882 
  src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 
7239a6a5eb5479e395e16423c83fdf80a77e5a83 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
4b50e2069407dc263b4fc93f1827d3a8836253bf 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
f806297d1d0700155c976743f936b2b8a3a390fb 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
769348e6b8a5c701734afff391b1c77de35222c6 
  
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 
22db80eaf34fe736fa5a3a9289836c9ac9e59906 
  
src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerFactory.java 
52b2e83efc0c289366b5246aff32553a546b1a70 
  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
5350ec945fbe028ee4641683815a068ce00b5efc 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
39729b374fe4e383f9b5ada7d016923766df9af7 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
7a8c3b882633376a1bf6a78616d55aaa7401d13f 
  
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/26478/diff/


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney

Re: Review Request 26383: Health Check Disabler

[David Pan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/
---

(Updated Oct. 9, 2014, 1:56 a.m.)


Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.


Changes
---

Removed default parameters from ThreadedHealthChecker


Repository: aurora


Description
---

The health check disabler allows health checks for a job to be snoozed 
temporarily by touching a snooze file in the job's sandbox.  The appropriate 
unit tests were modified/added.

The corresponding JIRA ticket is the following:
https://issues.apache.org/jira/browse/AURORA-795


Diffs (updated)
-

  src/main/python/apache/aurora/executor/common/health_checker.py 
4980411c847d12655cbb363404707ebd9f0bd163 
  src/test/python/apache/aurora/executor/common/BUILD 
c7f7a003c865d479ba6e3cd7b5349322f884f653 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
aa36415fa891fc523a3a376ffeca5d3cd5ceabec 

Diff: https://reviews.apache.org/r/26383/diff/


Testing
---

On vagrant in ~/aurora, I ran
./pants src/test/python/apache/aurora/executor::


Thanks,

David Pan

Re: Review Request 26383: Health Check Disabler

[Zameer Manji]

> On Oct. 8, 2014, 6:09 p.m., Zameer Manji wrote:
> > src/main/python/apache/aurora/executor/common/health_checker.py, line 41
> > 
> >
> > Why add a default value here?
> 
> David Pan wrote:
> The reason why ThreadedHealthChecker has defaults is because some tests 
> for ThreadedHealthChecker don't care about all the parameters to 
> ThreadedHealthChecker.  For example, if my test doesn't care about the 
> interval_secs, the defaults allow me to ignore that parameter altogether, and 
> focus on the parameters that I actually care about.

It isn't acceptable to alter parameters like that. Please modify your tests to 
pass in a value instead.


- Zameer


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/#review55924
---


On Oct. 8, 2014, 5:46 p.m., David Pan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/
> ---
> 
> (Updated Oct. 8, 2014, 5:46 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The health check disabler allows health checks for a job to be snoozed 
> temporarily by touching a snooze file in the job's sandbox.  The appropriate 
> unit tests were modified/added.
> 
> The corresponding JIRA ticket is the following:
> https://issues.apache.org/jira/browse/AURORA-795
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 4980411c847d12655cbb363404707ebd9f0bd163 
>   src/test/python/apache/aurora/executor/common/BUILD 
> c7f7a003c865d479ba6e3cd7b5349322f884f653 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec 
> 
> Diff: https://reviews.apache.org/r/26383/diff/
> 
> 
> Testing
> ---
> 
> On vagrant in ~/aurora, I ran
> ./pants src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> David Pan
> 
>

Re: Review Request 26383: Health Check Disabler

[David Pan]

> On Oct. 9, 2014, 1:09 a.m., Zameer Manji wrote:
> > src/main/python/apache/aurora/executor/common/health_checker.py, line 41
> > 
> >
> > Why add a default value here?

The reason why ThreadedHealthChecker has defaults is because some tests for 
ThreadedHealthChecker don't care about all the parameters to 
ThreadedHealthChecker.  For example, if my test doesn't care about the 
interval_secs, the defaults allow me to ignore that parameter altogether, and 
focus on the parameters that I actually care about.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/#review55924
---


On Oct. 9, 2014, 12:46 a.m., David Pan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/
> ---
> 
> (Updated Oct. 9, 2014, 12:46 a.m.)
> 
> 
> Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The health check disabler allows health checks for a job to be snoozed 
> temporarily by touching a snooze file in the job's sandbox.  The appropriate 
> unit tests were modified/added.
> 
> The corresponding JIRA ticket is the following:
> https://issues.apache.org/jira/browse/AURORA-795
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 4980411c847d12655cbb363404707ebd9f0bd163 
>   src/test/python/apache/aurora/executor/common/BUILD 
> c7f7a003c865d479ba6e3cd7b5349322f884f653 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec 
> 
> Diff: https://reviews.apache.org/r/26383/diff/
> 
> 
> Testing
> ---
> 
> On vagrant in ~/aurora, I ran
> ./pants src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> David Pan
> 
>

Re: Review Request 26383: Health Check Disabler

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/#review55924
---



src/main/python/apache/aurora/executor/common/health_checker.py


Why add a default value here?



src/main/python/apache/aurora/executor/common/health_checker.py


Why add a default value here?



src/main/python/apache/aurora/executor/common/health_checker.py


Why add a default here?


- Zameer Manji


On Oct. 8, 2014, 5:46 p.m., David Pan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/
> ---
> 
> (Updated Oct. 8, 2014, 5:46 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The health check disabler allows health checks for a job to be snoozed 
> temporarily by touching a snooze file in the job's sandbox.  The appropriate 
> unit tests were modified/added.
> 
> The corresponding JIRA ticket is the following:
> https://issues.apache.org/jira/browse/AURORA-795
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 4980411c847d12655cbb363404707ebd9f0bd163 
>   src/test/python/apache/aurora/executor/common/BUILD 
> c7f7a003c865d479ba6e3cd7b5349322f884f653 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec 
> 
> Diff: https://reviews.apache.org/r/26383/diff/
> 
> 
> Testing
> ---
> 
> On vagrant in ~/aurora, I ran
> ./pants src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> David Pan
> 
>

Re: Review Request 26383: Health Check Disabler

[David Pan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/
---

(Updated Oct. 9, 2014, 12:46 a.m.)


Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.


Repository: aurora


Description (updated)
---

The health check disabler allows health checks for a job to be snoozed 
temporarily by touching a snooze file in the job's sandbox.  The appropriate 
unit tests were modified/added.

The corresponding JIRA ticket is the following:
https://issues.apache.org/jira/browse/AURORA-795


Diffs
-

  src/main/python/apache/aurora/executor/common/health_checker.py 
4980411c847d12655cbb363404707ebd9f0bd163 
  src/test/python/apache/aurora/executor/common/BUILD 
c7f7a003c865d479ba6e3cd7b5349322f884f653 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
aa36415fa891fc523a3a376ffeca5d3cd5ceabec 

Diff: https://reviews.apache.org/r/26383/diff/


Testing
---

On vagrant in ~/aurora, I ran
./pants src/test/python/apache/aurora/executor::


Thanks,

David Pan

Re: Review Request 26383: Health Check Disabler

[David Pan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/
---

(Updated Oct. 9, 2014, 12:43 a.m.)


Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.


Changes
---

Removed configuration for snooze file and snooze duration.  Removed time 
control.  Addressed other comments.


Repository: aurora


Description
---

The health check disabler allows health checks for a job to be snoozed 
temporarily by touching a snooze file in the job's sandbox.  The path of the 
snooze file and the snooze duration can be set in the HealthCheckConfig.  The 
appropriate unit tests were modified/added.

The corresponding JIRA ticket is the following:
https://issues.apache.org/jira/browse/AURORA-795


Diffs (updated)
-

  src/main/python/apache/aurora/executor/common/health_checker.py 
4980411c847d12655cbb363404707ebd9f0bd163 
  src/test/python/apache/aurora/executor/common/BUILD 
c7f7a003c865d479ba6e3cd7b5349322f884f653 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
aa36415fa891fc523a3a376ffeca5d3cd5ceabec 

Diff: https://reviews.apache.org/r/26383/diff/


Testing
---

On vagrant in ~/aurora, I ran
./pants src/test/python/apache/aurora/executor::


Thanks,

David Pan

Re: Review Request 26469: Kill code to serve ApiBeta help pages that's no longer used now that the content is served directly.

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26469/#review55916
---

Ship it!


Ship It!

- Zameer Manji


On Oct. 8, 2014, 5:15 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26469/
> ---
> 
> (Updated Oct. 8, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Kill code to serve ApiBeta help pages that's no longer used now that the 
> content is served directly.
> 
> 
> Diffs
> -
> 
>   build.gradle 8f7eed0b58a00f8e5e0521512975087d5788b5b6 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
> eaf63382f689a045f837847736ef24fa75dee874 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 62154045f49c5b23949dc739d735c3e5d3680b89 
> 
> Diff: https://reviews.apache.org/r/26469/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> $ curl -I http://192.168.33.7:8081/apibeta/help/method/setQuota.html
> HTTP/1.1 200 OK
> Content-Type: text/html
> Vary: Accept-Encoding
> Content-Length: 0
> Server: Jetty(7.6.15.v20140411)
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26394: Deprecating Identity struct (renaming fields).

[Maxim Khutornenko]

> On Oct. 9, 2014, 12:05 a.m., Bill Farner wrote:
> > Thanks again for leading this - i'm very happy to see momentum on removing 
> > `Identity`.
> > 
> > Stepping back - i wonder if we should re-evaluate the way we do field 
> > deprecations now that we've established a protocol with JIRA and releases.  
> > This might mean we don't need to do the `DEPRECATED` mangling.  What do you 
> > think?

I still think visual code reminder is quite beneficial in avoiding the use of 
deprecated fields. The between-release time is just too long to remember what 
needs to be avoided when coding far away from thrift schema. Besides, it helps 
validate the concept and will facilitate later removal.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26394/#review55907
---


On Oct. 8, 2014, 11:39 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26394/
> ---
> 
> (Updated Oct. 8, 2014, 11:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Mark Chu-Carroll, and Bill 
> Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Part 1 of Identity struct deprecation: renaming fields.
> 
> 
> Diffs
> -
> 
>   examples/jobs/hello_world.aurora fc7877c3a60e56e301d9ee1fabd73446afca7236 
>   src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 
> 5c75cc8cae53edfa069c85c37ebad34774682081 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> e9f251508257cd7287ff00773e0073a3cd130df8 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> a76c3fac71b35115064fba6644cff0066fd9e630 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> eded7a59eb394748b93d7fbc085a1bdf64b043cc 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  865742171c11fbe5cf1469a69dd7258ec1be28c2 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> a0cb7bf56aeb7edd92b25d8d69a739d87452777a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 37176237fac336413267f3c8bb4e1b9a6255150c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5dcae4a6132026504cf02093ad4c68ab521e4ab8 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 288fb40f65629c8fd4eb7d92c8bf02369237de3b 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7b346e253677ee9b42c57782f7f67ff63b6a0083 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 7e9037ee921b009dc2b7c5adcf057bedebb01632 
>   src/main/resources/scheduler/assets/js/services.js 
> b744f375411e09b7f776e4a05ee5961227143439 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8794731f4b3f1033588bdfa33c292e4796319a2a 
>   src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java 
> e96974764844b5d1a3a05f6996075fccee209594 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 371ae87f5954fa5f092db1f6d21e2291d7576173 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 606c4434b7158220ccf1403b6deac939021fee31 
>   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
> f2d153f446247032ad9d8d173fb70870dbfdcca1 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
>   
> src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java
>  131bd826dfe47f40f3c27f29c095ed42953e316c 
>   src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java 
> efdde15939b2a851e38be53cceab395cc2cd82a1 
>   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
> 53d2c6bb78ad08a84639c1ecd48ba64d1

Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Joshua Cohen]

> On Oct. 8, 2014, 11:50 p.m., Joshua Cohen wrote:
> > This doesn't actually fail when I run it locally. I'm worried that we're 
> > just fixing a symptom of the problem, and not the problem itself (i.e. 
> > we're going to be adding random classes to the legacy list despite their 
> > actually having coverage).
> 
> Bill Farner wrote:
> I suspect you might be right.  I want to get to the bottom of this, since 
> i think the no-coverage check is really useful.  The next time we notice one 
> of these, let's remember to capture 
> `dist/reports/jacoco/test/jacocoTestReport.xml`.  That's what the check uses, 
> and it will help us determine if the check is faulty, or jacoco is doing 
> something odd.
> 
> Joshua Cohen wrote:
> In this case I think it is uncovering an actual case where coverage was 
> lost (because the static asset changes serve the html directly without going 
> through ApiBeta, I'll have a review out shortly to kill that code). That 
> being said, this check never failed for me locally which was super strange. I 
> had to run the tests locally in a debugger and confirm the breakpoint was not 
> hit.

https://reviews.apache.org/r/26469/


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/#review55900
---


On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26464/
> ---
> 
> (Updated Oct. 8, 2014, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-819
> https://issues.apache.org/jira/browse/AURORA-819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so 
> we blacklist it.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
> 
> Diff: https://reviews.apache.org/r/26464/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Review Request 26469: Kill code to serve ApiBeta help pages that's no longer used now that the content is served directly.

[Joshua Cohen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26469/
---

Review request for Aurora, Bill Farner and Zameer Manji.


Repository: aurora


Description
---

Kill code to serve ApiBeta help pages that's no longer used now that the 
content is served directly.


Diffs
-

  build.gradle 8f7eed0b58a00f8e5e0521512975087d5788b5b6 
  src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 
eaf63382f689a045f837847736ef24fa75dee874 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
62154045f49c5b23949dc739d735c3e5d3680b89 

Diff: https://reviews.apache.org/r/26469/diff/


Testing
---

./gradlew build -Pq

$ curl -I http://192.168.33.7:8081/apibeta/help/method/setQuota.html
HTTP/1.1 200 OK
Content-Type: text/html
Vary: Accept-Encoding
Content-Length: 0
Server: Jetty(7.6.15.v20140411)


Thanks,

Joshua Cohen

Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py

[Joe Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26431/#review55911
---

Ship it!


Thanks!

- Joe Smith


On Oct. 8, 2014, 4:46 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26431/
> ---
> 
> (Updated Oct. 8, 2014, 4:46 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-806
> https://issues.apache.org/jira/browse/AURORA-806
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moving post_drain script functionality into host_maintenance.py to support 
> per batch execution.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 9c2a9f77109791da574e1624d27b6b7096a2678e 
>   src/main/python/apache/aurora/client/commands/maintenance.py 
> e465d973e9f764076e18491e1691d44303c0f388 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> 40228df59e43bc6034f2dc651c166a0c4b78aea8 
> 
> Diff: https://reviews.apache.org/r/26431/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Joshua Cohen]

> On Oct. 8, 2014, 11:50 p.m., Joshua Cohen wrote:
> > This doesn't actually fail when I run it locally. I'm worried that we're 
> > just fixing a symptom of the problem, and not the problem itself (i.e. 
> > we're going to be adding random classes to the legacy list despite their 
> > actually having coverage).
> 
> Bill Farner wrote:
> I suspect you might be right.  I want to get to the bottom of this, since 
> i think the no-coverage check is really useful.  The next time we notice one 
> of these, let's remember to capture 
> `dist/reports/jacoco/test/jacocoTestReport.xml`.  That's what the check uses, 
> and it will help us determine if the check is faulty, or jacoco is doing 
> something odd.

In this case I think it is uncovering an actual case where coverage was lost 
(because the static asset changes serve the html directly without going through 
ApiBeta, I'll have a review out shortly to kill that code). That being said, 
this check never failed for me locally which was super strange. I had to run 
the tests locally in a debugger and confirm the breakpoint was not hit.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/#review55900
---


On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26464/
> ---
> 
> (Updated Oct. 8, 2014, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-819
> https://issues.apache.org/jira/browse/AURORA-819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so 
> we blacklist it.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
> 
> Diff: https://reviews.apache.org/r/26464/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26458: Adding wait loop into host_drain status monitoring.

[Tobias Weingartner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26458/#review55909
---



src/main/python/apache/aurora/admin/host_maintenance.py


Will this abort aurora_admin?

If so, writing these to the failed set is more desirable, along with 
continuing.


- Tobias Weingartner


On Oct. 8, 2014, 9:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26458/
> ---
> 
> (Updated Oct. 8, 2014, 9:40 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Brian Wickman.
> 
> 
> Bugs: AURORA-820
> https://issues.apache.org/jira/browse/AURORA-820
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Throttling status check calls now at a predefined 5 second interval with a 
> max timeout of 5 minutes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 9c2a9f77109791da574e1624d27b6b7096a2678e 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> 40228df59e43bc6034f2dc651c166a0c4b78aea8 
> 
> Diff: https://reviews.apache.org/r/26458/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26458: Adding wait loop into host_drain status monitoring.

[Joe Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26458/#review55899
---



src/main/python/apache/aurora/admin/host_maintenance.py


Is this provided so external modules can interact with the maintenance?



src/main/python/apache/aurora/admin/host_maintenance.py


move out to its own method?



src/main/python/apache/aurora/admin/host_maintenance.py


Maybe just raise a normal exception instead? :)

(and maybe log which hosts weren't moved?)



src/test/python/apache/aurora/admin/test_host_maintenance.py


can you assert on how many times this was called?


- Joe Smith


On Oct. 8, 2014, 2:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26458/
> ---
> 
> (Updated Oct. 8, 2014, 2:40 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Brian Wickman.
> 
> 
> Bugs: AURORA-820
> https://issues.apache.org/jira/browse/AURORA-820
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Throttling status check calls now at a predefined 5 second interval with a 
> max timeout of 5 minutes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 9c2a9f77109791da574e1624d27b6b7096a2678e 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> 40228df59e43bc6034f2dc651c166a0c4b78aea8 
> 
> Diff: https://reviews.apache.org/r/26458/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26394: Deprecating Identity struct (renaming fields).

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26394/#review55907
---


Thanks again for leading this - i'm very happy to see momentum on removing 
`Identity`.

Stepping back - i wonder if we should re-evaluate the way we do field 
deprecations now that we've established a protocol with JIRA and releases.  
This might mean we don't need to do the `DEPRECATED` mangling.  What do you 
think?

- Bill Farner


On Oct. 8, 2014, 11:39 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26394/
> ---
> 
> (Updated Oct. 8, 2014, 11:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Mark Chu-Carroll, and Bill 
> Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Part 1 of Identity struct deprecation: renaming fields.
> 
> 
> Diffs
> -
> 
>   examples/jobs/hello_world.aurora fc7877c3a60e56e301d9ee1fabd73446afca7236 
>   src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 
> 5c75cc8cae53edfa069c85c37ebad34774682081 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> e9f251508257cd7287ff00773e0073a3cd130df8 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> a76c3fac71b35115064fba6644cff0066fd9e630 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> eded7a59eb394748b93d7fbc085a1bdf64b043cc 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  865742171c11fbe5cf1469a69dd7258ec1be28c2 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> a0cb7bf56aeb7edd92b25d8d69a739d87452777a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 37176237fac336413267f3c8bb4e1b9a6255150c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5dcae4a6132026504cf02093ad4c68ab521e4ab8 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 288fb40f65629c8fd4eb7d92c8bf02369237de3b 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7b346e253677ee9b42c57782f7f67ff63b6a0083 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 7e9037ee921b009dc2b7c5adcf057bedebb01632 
>   src/main/resources/scheduler/assets/js/services.js 
> b744f375411e09b7f776e4a05ee5961227143439 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8794731f4b3f1033588bdfa33c292e4796319a2a 
>   src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java 
> e96974764844b5d1a3a05f6996075fccee209594 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 371ae87f5954fa5f092db1f6d21e2291d7576173 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 606c4434b7158220ccf1403b6deac939021fee31 
>   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
> f2d153f446247032ad9d8d173fb70870dbfdcca1 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
>   
> src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java
>  131bd826dfe47f40f3c27f29c095ed42953e316c 
>   src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java 
> efdde15939b2a851e38be53cceab395cc2cd82a1 
>   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
> 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 6534329a92bf005223fa8907cbe4a8a3a511e142 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 9970948bace4c0ecbc51d6fc79270d77fb17bf87 
>   src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> b8b9c1b8cdf7b641976c583a71fdd9b0c14e6e5f 
> 

Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Bill Farner]

> On Oct. 8, 2014, 11:50 p.m., Joshua Cohen wrote:
> > This doesn't actually fail when I run it locally. I'm worried that we're 
> > just fixing a symptom of the problem, and not the problem itself (i.e. 
> > we're going to be adding random classes to the legacy list despite their 
> > actually having coverage).

I suspect you might be right.  I want to get to the bottom of this, since i 
think the no-coverage check is really useful.  The next time we notice one of 
these, let's remember to capture 
`dist/reports/jacoco/test/jacocoTestReport.xml`.  That's what the check uses, 
and it will help us determine if the check is faulty, or jacoco is doing 
something odd.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/#review55900
---


On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26464/
> ---
> 
> (Updated Oct. 8, 2014, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-819
> https://issues.apache.org/jira/browse/AURORA-819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so 
> we blacklist it.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
> 
> Diff: https://reviews.apache.org/r/26464/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.

[Mark Chu-Carroll]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26363/
---

(Updated Oct. 8, 2014, 7:56 p.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

rebase


Bugs: aurora-792
https://issues.apache.org/jira/browse/aurora-792


Repository: aurora


Description
---

Make the large-update check in the client update command consider instance 
parameters.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/context.py 
4e94a4e3017f3b2ace38d6d8ce68c3c778c8cd0e 
  src/main/python/apache/aurora/client/cli/jobs.py 
0277cbed4b7eb927d6e5823b4b41d90b366f81d0 
  src/test/python/apache/aurora/client/cli/BUILD 
d33e86643a59879c115876c98bd1dc19aa7ae61c 
  src/test/python/apache/aurora/client/cli/test_update.py 
cff1b6578aec6f5bcc1e610e58b47af233f32b41 

Diff: https://reviews.apache.org/r/26363/diff/


Testing
---

New unit tests added, all test pass.


Thanks,

Mark Chu-Carroll

Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Joshua Cohen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/#review55900
---


This doesn't actually fail when I run it locally. I'm worried that we're just 
fixing a symptom of the problem, and not the problem itself (i.e. we're going 
to be adding random classes to the legacy list despite their actually having 
coverage).

- Joshua Cohen


On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26464/
> ---
> 
> (Updated Oct. 8, 2014, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-819
> https://issues.apache.org/jira/browse/AURORA-819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so 
> we blacklist it.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
> 
> Diff: https://reviews.apache.org/r/26464/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py

[Maxim Khutornenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26431/
---

(Updated Oct. 8, 2014, 11:46 p.m.)


Review request for Aurora, Joe Smith and Mark Chu-Carroll.


Changes
---

CR comments.


Bugs: AURORA-806
https://issues.apache.org/jira/browse/AURORA-806


Repository: aurora


Description
---

Moving post_drain script functionality into host_maintenance.py to support per 
batch execution.


Diffs (updated)
-

  src/main/python/apache/aurora/admin/host_maintenance.py 
9c2a9f77109791da574e1624d27b6b7096a2678e 
  src/main/python/apache/aurora/client/commands/maintenance.py 
e465d973e9f764076e18491e1691d44303c0f388 
  src/test/python/apache/aurora/admin/test_host_maintenance.py 
40228df59e43bc6034f2dc651c166a0c4b78aea8 

Diff: https://reviews.apache.org/r/26431/diff/


Testing
---

./pants src/test/python:all


Thanks,

Maxim Khutornenko

Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py

[Maxim Khutornenko]

> On Oct. 8, 2014, 7:08 p.m., Joe Smith wrote:
> > src/test/python/apache/aurora/client/commands/test_maintenance.py, line 117
> > 
> >
> > can this be a mock that we assert gets called 3 times?
> > 
> > (This test is a little bit of an integration test since it goes in and 
> > is also testing perform_maintenace, but a full refactor of this test may be 
> > too much for now)

Sure, reverted it.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26431/#review55859
---


On Oct. 8, 2014, 12:13 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26431/
> ---
> 
> (Updated Oct. 8, 2014, 12:13 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-806
> https://issues.apache.org/jira/browse/AURORA-806
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moving post_drain script functionality into host_maintenance.py to support 
> per batch execution.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 9c2a9f77109791da574e1624d27b6b7096a2678e 
>   src/main/python/apache/aurora/client/commands/maintenance.py 
> e465d973e9f764076e18491e1691d44303c0f388 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> 40228df59e43bc6034f2dc651c166a0c4b78aea8 
>   src/test/python/apache/aurora/client/commands/test_maintenance.py 
> d86aaf677804301fa5ddf1f76dba552f4fafb8c3 
> 
> Diff: https://reviews.apache.org/r/26431/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Bill Farner]

> On Oct. 8, 2014, 11:31 p.m., Bill Farner wrote:
> > Ship It!

This is now on master:

$ git log -1 --abbrev-commit origin/master
commit 18ae905
Author: Zameer Manji 
Date:   Wed Oct 8 16:33:42 2014 -0700

Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no 
coverage.

Bugs closed: AURORA-819

Reviewed at https://reviews.apache.org/r/26464/


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/#review55894
---


On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26464/
> ---
> 
> (Updated Oct. 8, 2014, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-819
> https://issues.apache.org/jira/browse/AURORA-819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so 
> we blacklist it.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
> 
> Diff: https://reviews.apache.org/r/26464/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26394: Deprecating Identity struct (renaming fields).

[Maxim Khutornenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26394/
---

(Updated Oct. 8, 2014, 11:39 p.m.)


Review request for Aurora, David McLaughlin, Mark Chu-Carroll, and Bill Farner.


Changes
---

Rebased.

Ping.


Bugs: AURORA-84
https://issues.apache.org/jira/browse/AURORA-84


Repository: aurora


Description (updated)
---

Part 1 of Identity struct deprecation: renaming fields.


Diffs (updated)
-

  examples/jobs/hello_world.aurora fc7877c3a60e56e301d9ee1fabd73446afca7236 
  src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 
5c75cc8cae53edfa069c85c37ebad34774682081 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
e9f251508257cd7287ff00773e0073a3cd130df8 
  src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
a76c3fac71b35115064fba6644cff0066fd9e630 
  src/main/java/org/apache/aurora/scheduler/base/Query.java 
eded7a59eb394748b93d7fbc085a1bdf64b043cc 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 865742171c11fbe5cf1469a69dd7258ec1be28c2 
  src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
a0cb7bf56aeb7edd92b25d8d69a739d87452777a 
  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
37176237fac336413267f3c8bb4e1b9a6255150c 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5dcae4a6132026504cf02093ad4c68ab521e4ab8 
  src/main/python/apache/aurora/client/api/instance_watcher.py 
b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
  src/main/python/apache/aurora/client/api/sla.py 
b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
  src/main/python/apache/aurora/client/cli/task.py 
c41484bdc27266443bc4e139e1ebb362a59be0f9 
  src/main/python/apache/aurora/client/commands/admin.py 
deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
  src/main/python/apache/aurora/client/commands/core.py 
58f419e674f1a9a0ae9da6faa2e39c8167bab597 
  src/main/python/apache/aurora/client/commands/ssh.py 
d2b8bf675556b924d3d63b545d036dc48a081486 
  src/main/python/apache/aurora/config/thrift.py 
288fb40f65629c8fd4eb7d92c8bf02369237de3b 
  src/main/python/apache/aurora/executor/aurora_executor.py 
2c6423d096656f426a4385f4edef6875ebad7049 
  src/main/python/apache/aurora/executor/common/announcer.py 
74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
7b346e253677ee9b42c57782f7f67ff63b6a0083 
  src/main/resources/scheduler/assets/js/controllers.js 
7e9037ee921b009dc2b7c5adcf057bedebb01632 
  src/main/resources/scheduler/assets/js/services.js 
b744f375411e09b7f776e4a05ee5961227143439 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
8794731f4b3f1033588bdfa33c292e4796319a2a 
  src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java 
e96974764844b5d1a3a05f6996075fccee209594 
  src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
371ae87f5954fa5f092db1f6d21e2291d7576173 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
606c4434b7158220ccf1403b6deac939021fee31 
  src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
f2d153f446247032ad9d8d173fb70870dbfdcca1 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
  
src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java
 131bd826dfe47f40f3c27f29c095ed42953e316c 
  src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java 
efdde15939b2a851e38be53cceab395cc2cd82a1 
  src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
6534329a92bf005223fa8907cbe4a8a3a511e142 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
9970948bace4c0ecbc51d6fc79270d77fb17bf87 
  src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
b8b9c1b8cdf7b641976c583a71fdd9b0c14e6e5f 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 ee9587582bd7c45a446e8afe28930c18a97d2792 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
f3c7c5bd53df759432beda4fa46db49fd0514b42 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
d2d3e86bb5acf3402f55188b9ae440412ef14b5a 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
6a9c4ee278ed3ee8222404504e571f20991c2ae2 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
62154045f49c5b23949dc739d735c3e5d3680b89 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b58c8f363e6e7c72accaf590b2a7cb7bf24275ea 
  src/test/java/org/apache/aurora/scheduler/sla/Sl

Re: Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/#review55894
---

Ship it!


Ship It!

- Bill Farner


On Oct. 8, 2014, 11:09 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26464/
> ---
> 
> (Updated Oct. 8, 2014, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-819
> https://issues.apache.org/jira/browse/AURORA-819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so 
> we blacklist it.
> 
> 
> Diffs
> -
> 
>   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
> 
> Diff: https://reviews.apache.org/r/26464/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Review Request 26464: Blacklist org/apache/aurora/scheduler/http/api/ApiBeta$2 which has no coverage.

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26464/
---

Review request for Aurora, Joshua Cohen and Bill Farner.


Bugs: AURORA-819
https://issues.apache.org/jira/browse/AURORA-819


Repository: aurora


Description
---

The org/apache/aurora/scheduler/http/api/ApiBeta$2 class has no coverage so we 
blacklist it.


Diffs
-

  build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 

Diff: https://reviews.apache.org/r/26464/diff/


Testing
---

./gradlew clean build


Thanks,

Zameer Manji

Re: Review Request 26376: Implementing non-prod MTTA/R SLA metrics.

[Maxim Khutornenko]

> On Oct. 8, 2014, 5:32 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, line 81
> > 
> >
> > Seems like we're moving towards identical calculations for prod and 
> > non-prod tasks.  Should we just do that now and avoid the independent 
> > groupings?

I don't necessarily see prods and non-prods converging any time soon. Doing 
that now would require more work to exclude non-prods from platform uptime 
calculations and would add redundant job uptime metrics. I'd rather keep them 
separate at least for now and see if it ever becomes a trend.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26376/#review55829
---


On Oct. 6, 2014, 7:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26376/
> ---
> 
> (Updated Oct. 6, 2014, 7:10 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-774
> https://issues.apache.org/jira/browse/AURORA-774
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implementing non-prod MTTA/R SLA metrics.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java 
> dca680415426be2bc760875d5774c9e9399ea94b 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
> 33b6bbe4b39d516595276128bbfa0686d0bb9cbd 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> aeb90bbb822254cbe4691e45092b9581596ad800 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
> 96a04389e6b14c4d1c0bdb4d06abc046e7ea2151 
> 
> Diff: https://reviews.apache.org/r/26376/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26383: Health Check Disabler

[David Pan]

> On Oct. 6, 2014, 10:55 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/executor/common/health_checker.py, lines 
> > 157-158
> > 
> >
> > I know you didn't originate this pattern, but it seems odd for these 
> > defaults to be repeated on both the HealthChecker and the 
> > ThreadedHealthChecker. Perhaps they should be constants, or maybe 
> > ThreadedHealthChecker shouldn't have defaults at all?

I actually did originate this pattern.  The reason why ThreadedHealthChecker 
has defaults is because some tests don't care about all the parameters to 
ThreadedHealthChecker.  For example, if my test doesn't care about the 
interval_secs, the defaults allow me to ignore that parameter altogether, and 
focus on the parameters that I actually care about.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/#review55590
---


On Oct. 6, 2014, 9:24 p.m., David Pan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/
> ---
> 
> (Updated Oct. 6, 2014, 9:24 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The health check disabler allows health checks for a job to be snoozed 
> temporarily by touching a snooze file in the job's sandbox.  The path of the 
> snooze file and the snooze duration can be set in the HealthCheckConfig.  The 
> appropriate unit tests were modified/added.
> 
> The corresponding JIRA ticket is the following:
> https://issues.apache.org/jira/browse/AURORA-795
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
>   docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 
>   src/main/python/apache/aurora/config/schema/base.py 
> f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 4980411c847d12655cbb363404707ebd9f0bd163 
>   src/test/python/apache/aurora/executor/common/BUILD 
> c7f7a003c865d479ba6e3cd7b5349322f884f653 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec 
> 
> Diff: https://reviews.apache.org/r/26383/diff/
> 
> 
> Testing
> ---
> 
> On vagrant in ~/aurora, I ran
> ./pants src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> David Pan
> 
>

Re: Review Request 26425: Fixing quota checking for updates.

[Maxim Khutornenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26425/
---

(Updated Oct. 8, 2014, 10:58 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

CR comments.
-kevints per request.


Bugs: AURORA-802
https://issues.apache.org/jira/browse/AURORA-802


Repository: aurora


Description
---

Rolling up prod consumption does not work for quota checking during job update 
intitiation where per-instance filtering is required. Splitting the 
QuotaManager interface into 2 use cases:
- createJob/addInstances
- startJobUpdate

Reverting the IResourceAggregate abstraction in QuotaManager in favor of 
task/update objects to simplify handling.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
2442b06f318c8d0cefe60296950baa1b916b42f7 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java 
d197dd515eb646cb4babadf22e9cf6480a919060 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5dcae4a6132026504cf02093ad4c68ab521e4ab8 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b58c8f363e6e7c72accaf590b2a7cb7bf24275ea 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 

Diff: https://reviews.apache.org/r/26425/diff/


Testing
---

./gradlew -Pq build
Also, tested various update scenarios in vagrant.


Thanks,

Maxim Khutornenko

Re: Review Request 26425: Fixing quota checking for updates.

[Maxim Khutornenko]

> On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 90
> > 
> >
> > These methods read strangely to me: "check the quota of this task 
> > config", and "check the quota of this update".
> > 
> > How about `checkInstanceAddition`, and `checkJobUpdate`?

Sure. Done.


> On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 148
> > 
> >
> > checkArgument(instances >= 0)
> > 
> > (technically we could assert > 0, but this function should not care if 
> > instances == 0)

Done.


> On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 168
> > 
> >
> > The method names in QuotaInfo make this pretty confusing.  It's not 
> > obvious why `getQuota()` and `getProdConsumpgion()` are used the way they 
> > are.  To me this suggests the method names should be re-evaluated to avoid 
> > misuse.

The getQuota() is exactly what it is. I am not sure I have a better alternative 
for getProdConsumption() without using something stupidly long. Technically, it 
is a prod consumption in pure (IJobUpdate absent) or in a adjusted form where a 
not-yet-saved update is added to the prod mix to simplify handling. I have 
added javadoc comments for the getQuotaInfo() to hopefully make it clear.


> On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 209
> > 
> >
> > I would find this easier if functions were composed rather than 
> > combined.  e.g.:
> > 
> > // Fetch the active production tasks in a role, group by job, 
> > compute resources.
> > Map getProdTaskResources(String role);
> > 
> > // Call getProdTaskResources, combine values.
> > IResourceAggregate getProdConsumption(String role);
> > 
> > // Call getProdTaskResources, calculate charge for updated job, 
> > combine values.
> > IResourceAggregate getProdConsumptionDuringUpdate(IJobUpdate 
> > update);

The above would not quite work as the updates must be checked against while the 
prod consumption is computed to yield the correct value. Hence the combination 
rather than composition.


> On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 317
> > 
> >
> > Avoid encoding type information in the method name.  Also, observe Law 
> > of Demeter.  How about:
> > 
> > private static IResourceAggregate 
> > toProdResources(IJobUpdateInstructions instructions)

Type encoding (Job Update) here is unintentional. Changed the name anyway.


> On Oct. 8, 2014, 5:17 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 320
> > 
> >
> > This is well-suited for a javadoc.

lol, it was actually a javadoc in QuotaUtil. Fixed.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26425/#review55819
---


On Oct. 7, 2014, 10:03 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26425/
> ---
> 
> (Updated Oct. 7, 2014, 10:03 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-802
> https://issues.apache.org/jira/browse/AURORA-802
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Rolling up prod consumption does not work for quota checking during job 
> update intitiation where per-instance filtering is required. Splitting the 
> QuotaManager interface into 2 use cases:
> - createJob/addInstances
> - startJobUpdate
> 
> Reverting the IResourceAggregate abstraction in QuotaManager in favor of 
> task/update objects to simplify handling.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 2442b06f318c8d0cefe60296950baa1b916b42f7 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java 
> d197dd515eb646cb4babadf22e9cf6480a919060 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5dcae4a6132026504cf02093ad4c68ab521e4ab8 
>   src/test/java/org/apach

Re: Review Request 26430: Remove deprecated configuration options.

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26430/#review55885
---

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 8, 2014, 3:29 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26430/
> ---
> 
> (Updated Oct. 8, 2014, 3:29 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman.
> 
> 
> Bugs: AURORA-333
> https://issues.apache.org/jira/browse/AURORA-333
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This removes several long deprecated configuration options from aurora.
> 
> The features removed are automatically filling out environment, cron_policy, 
> daemon, health_check_interval_secs, recipes and the PackerObject.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  865742171c11fbe5cf1469a69dd7258ec1be28c2 
>   src/main/python/apache/aurora/client/binding_helper.py 
> 6d6a06785c6840e4345e304eb4e242682676ac66 
>   src/main/python/apache/aurora/client/config.py 
> e440f587d100ce46b2df85ccc663912c615051ef 
>   src/main/python/apache/aurora/config/recipes.py 
> 68b5d252f87a592d4c2f7d52525163829bea2cc9 
>   src/main/python/apache/aurora/config/schema/base.py 
> f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
>   src/main/python/apache/aurora/config/thrift.py 
> 288fb40f65629c8fd4eb7d92c8bf02369237de3b 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8794731f4b3f1033588bdfa33c292e4796319a2a 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  ee9587582bd7c45a446e8afe28930c18a97d2792 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> d3c90416e48e25d83f28f966b21c018ee2a1ac27 
>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
> cdd29ea2b6fc92b967571028d299260556e16d42 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1835843f1795b0530874ec561582df17acfbce65 
>   src/test/python/apache/aurora/client/cli/util.py 
> 967b5dcdab76f0581f6d2a518409c30de0800f27 
>   src/test/python/apache/aurora/client/commands/util.py 
> 663f2f4a16113a36826943b7238cad900ae0dcd2 
>   src/test/python/apache/aurora/client/test_config.py 
> 901c3378ed59c44b7e2dea239f186193f1f66355 
>   src/test/python/apache/aurora/config/test_thrift.py 
> fd28313df2cfd5a9c7d00f6d329518b4caabacb2 
> 
> Diff: https://reviews.apache.org/r/26430/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26430: Remove deprecated configuration options.

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26430/
---

(Updated Oct. 8, 2014, 3:29 p.m.)


Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman.


Bugs: AURORA-333
https://issues.apache.org/jira/browse/AURORA-333


Repository: aurora


Description (updated)
---

This removes several long deprecated configuration options from aurora.

The features removed are automatically filling out environment, cron_policy, 
daemon, health_check_interval_secs, recipes and the PackerObject.


Diffs
-

  docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 865742171c11fbe5cf1469a69dd7258ec1be28c2 
  src/main/python/apache/aurora/client/binding_helper.py 
6d6a06785c6840e4345e304eb4e242682676ac66 
  src/main/python/apache/aurora/client/config.py 
e440f587d100ce46b2df85ccc663912c615051ef 
  src/main/python/apache/aurora/config/recipes.py 
68b5d252f87a592d4c2f7d52525163829bea2cc9 
  src/main/python/apache/aurora/config/schema/base.py 
f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
  src/main/python/apache/aurora/config/thrift.py 
288fb40f65629c8fd4eb7d92c8bf02369237de3b 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
8794731f4b3f1033588bdfa33c292e4796319a2a 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 ee9587582bd7c45a446e8afe28930c18a97d2792 
  src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
d3c90416e48e25d83f28f966b21c018ee2a1ac27 
  src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
cdd29ea2b6fc92b967571028d299260556e16d42 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1835843f1795b0530874ec561582df17acfbce65 
  src/test/python/apache/aurora/client/cli/util.py 
967b5dcdab76f0581f6d2a518409c30de0800f27 
  src/test/python/apache/aurora/client/commands/util.py 
663f2f4a16113a36826943b7238cad900ae0dcd2 
  src/test/python/apache/aurora/client/test_config.py 
901c3378ed59c44b7e2dea239f186193f1f66355 
  src/test/python/apache/aurora/config/test_thrift.py 
fd28313df2cfd5a9c7d00f6d329518b4caabacb2 

Diff: https://reviews.apache.org/r/26430/diff/


Testing
---

./build-support/jenkins/build.sh


Thanks,

Zameer Manji

Re: Review Request 26430: Remove deprecated configuration options.

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26430/
---

(Updated Oct. 8, 2014, 3:29 p.m.)


Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman.


Changes
---

Brian's feedback.


Bugs: AURORA-333
https://issues.apache.org/jira/browse/AURORA-333


Repository: aurora


Description
---

This removes several long deprecated configuration options from aurora.

The features removed are cron_policy, daemon, health_check_interval_secs, 
recipes and the PackerObject


Diffs (updated)
-

  docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 865742171c11fbe5cf1469a69dd7258ec1be28c2 
  src/main/python/apache/aurora/client/binding_helper.py 
6d6a06785c6840e4345e304eb4e242682676ac66 
  src/main/python/apache/aurora/client/config.py 
e440f587d100ce46b2df85ccc663912c615051ef 
  src/main/python/apache/aurora/config/recipes.py 
68b5d252f87a592d4c2f7d52525163829bea2cc9 
  src/main/python/apache/aurora/config/schema/base.py 
f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
  src/main/python/apache/aurora/config/thrift.py 
288fb40f65629c8fd4eb7d92c8bf02369237de3b 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
8794731f4b3f1033588bdfa33c292e4796319a2a 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 ee9587582bd7c45a446e8afe28930c18a97d2792 
  src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
d3c90416e48e25d83f28f966b21c018ee2a1ac27 
  src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
cdd29ea2b6fc92b967571028d299260556e16d42 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1835843f1795b0530874ec561582df17acfbce65 
  src/test/python/apache/aurora/client/cli/util.py 
967b5dcdab76f0581f6d2a518409c30de0800f27 
  src/test/python/apache/aurora/client/commands/util.py 
663f2f4a16113a36826943b7238cad900ae0dcd2 
  src/test/python/apache/aurora/client/test_config.py 
901c3378ed59c44b7e2dea239f186193f1f66355 
  src/test/python/apache/aurora/config/test_thrift.py 
fd28313df2cfd5a9c7d00f6d329518b4caabacb2 

Diff: https://reviews.apache.org/r/26430/diff/


Testing
---

./build-support/jenkins/build.sh


Thanks,

Zameer Manji

Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.

[Joshua Cohen]

> On Oct. 6, 2014, 4:44 p.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/client/cli/test_update.py, lines 342-353
> > 
> >
> > This sequence of mocks and writing config to a file is repeated in... 
> > many (all?) of these tests. Can you refactor to remove the repetition?
> 
> Mark Chu-Carroll wrote:
> I really think that that should not be done.
> 
> For tests, I really like to be able to see, in an instant, exactly what 
> mocks are being used by a particular test.  I don't want to have to look 
> somewhere else; I don't want to have to mentally combine the stuff that's 
> mocked in a common frame and the different stuff that's mocked and/or 
> reconfigured in a particular test. The test should be as clear as it can be, 
> and anything from the environment that it's mucking with should be done 
> explicitly in the most local-possible context.

Not a ship blocker for me, just my perspective, but these mocks are identical 
in several calls, if many client test cases need to mock the exact same set of 
things in the exact same way, it seems worthwhile to simplify that process.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26363/#review55512
---


On Oct. 8, 2014, 5:40 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26363/
> ---
> 
> (Updated Oct. 8, 2014, 5:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-792
> https://issues.apache.org/jira/browse/aurora-792
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Make the large-update check in the client update command consider instance 
> parameters.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/context.py 
> 301531fcb443297facb78d87a18073c8b7fd4064 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 103fb53cb5101d3e025d8712e3887bccdfbb6aeb 
>   src/test/python/apache/aurora/client/cli/BUILD 
> 8ce6bd5b7faa1579372fb6935180ea982af64af8 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 85b1db19d89967a741bfba7964eeb368426f0b61 
> 
> Diff: https://reviews.apache.org/r/26363/diff/
> 
> 
> Testing
> ---
> 
> New unit tests added, all test pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Review Request 26458: Adding wait loop into host_drain status monitoring.

[Maxim Khutornenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26458/
---

Review request for Aurora, Joe Smith and Brian Wickman.


Bugs: AURORA-820
https://issues.apache.org/jira/browse/AURORA-820


Repository: aurora


Description
---

Throttling status check calls now at a predefined 5 second interval with a max 
timeout of 5 minutes.


Diffs
-

  src/main/python/apache/aurora/admin/host_maintenance.py 
9c2a9f77109791da574e1624d27b6b7096a2678e 
  src/test/python/apache/aurora/admin/test_host_maintenance.py 
40228df59e43bc6034f2dc651c166a0c4b78aea8 

Diff: https://reviews.apache.org/r/26458/diff/


Testing
---

./pants src/test/python:all


Thanks,

Maxim Khutornenko

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26308/
---

(Updated Oct. 8, 2014, 8:57 p.m.)


Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.


Changes
---

Another rebase cycle, more merge conflicts while waiting for tests to run.


Repository: aurora


Description
---

Fixes two problems:

- mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
objects were being passed around and somehow resulted in the test case passing.
- use of `threading.Event()` was broken in scheduler_client.py.  I don't think 
it's possible to enter those branches.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/__init__.py 
26300792594e4005dacc139a9f89711b8a66ab61 
  src/main/python/apache/aurora/client/api/command_runner.py 
a1fed5fc75dde3a79c840515e6daa4741156ef97 
  src/main/python/apache/aurora/client/api/job_monitor.py 
18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
311c954f1db245b75192d00c6aca0721085fbf32 
  src/main/python/apache/aurora/client/api/updater.py 
bb4a7555851341e450da408441975f078f2b5576 
  src/main/python/apache/aurora/common/aurora_job_key.py 
a7ca7b6df6b9566b3ce617283ccac948deb2eb83 
  src/test/python/apache/aurora/client/api/test_job_monitor.py 
5b26539f86a0a82f72753a803a769eda77cbc332 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1835843f1795b0530874ec561582df17acfbce65 
  src/test/python/apache/aurora/client/api/test_updater.py 
fe27391a7b267e5acf4dc5f1a82a8199777d5b04 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
733327b557df3e81c6d723bf24e753442fa375ca 
  src/test/python/apache/aurora/client/cli/test_cancel_update.py 
c15e142930c9474c7873dd931261b6ab4eb5967f 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 
  src/test/python/apache/aurora/client/cli/test_create.py 
d17203578438fcae1f12b5e70e78e420b87554e4 
  src/test/python/apache/aurora/client/cli/test_diff.py 
e1a6f764830e06c73d0bc10a3b5da67219da836c 
  src/test/python/apache/aurora/client/cli/test_kill.py 
e3a366bf67074e50787394cad58d5e01359b641e 
  src/test/python/apache/aurora/client/cli/test_logging.py 
6285fbb07442291c2dc4096e68eb285c98994097 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
7c79c7a75e77e47544684c0f767d248d2540901a 
  src/test/python/apache/aurora/client/cli/test_status.py 
d26371249aa4f5b671c9ace9b503e756cb39528a 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
1ce9a632874e818eee71573cd481842affae3615 
  src/test/python/apache/aurora/client/cli/test_update.py 
85b1db19d89967a741bfba7964eeb368426f0b61 
  src/test/python/apache/aurora/client/commands/test_admin.py 
1192556c027dc3adf16bb37adeac7798cf9ef93d 
  src/test/python/apache/aurora/client/commands/test_cancel_update.py 
5f05ef7c0643d189de3de38c75aae58c2a3814a4 
  src/test/python/apache/aurora/client/commands/test_create.py 
7503345ea1c0f224a894ce02cc2c2d8719574e32 
  src/test/python/apache/aurora/client/commands/test_diff.py 
8f5da7d2bca9b0486b635afe49d3885151624e12 
  src/test/python/apache/aurora/client/commands/test_hooks.py 
0861f13b13a8406950ba953efba0ffae186a8253 
  src/test/python/apache/aurora/client/commands/test_kill.py 
c0a6fd44c5691cde50746ffdec325bb11a33469a 
  src/test/python/apache/aurora/client/commands/test_run.py 
e97b5156609f80a4170028e780bcd891e56983ff 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
33262248d14a2cc182d87fde736090f38c322bf4 
  src/test/python/apache/aurora/client/commands/test_status.py 
bda1f28d544ae48428129f8167d8632ef27f5fba 
  src/test/python/apache/aurora/client/commands/test_update.py 
af2cbc7f88287201a472ba36902b00d90bc77d3b 

Diff: https://reviews.apache.org/r/26308/diff/


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26430: Remove deprecated configuration options.

[Brian Wickman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26430/#review55870
---



src/main/python/apache/aurora/client/config.py


default environments are also deprecated and should be removed.



src/main/python/apache/aurora/config/thrift.py


cron_policy shouldn't be empty right?  since it's a Default() field.



src/main/python/apache/aurora/config/thrift.py


same -- service is a Default(Boolean, False) so has_service() should never 
be false.


- Brian Wickman


On Oct. 8, 2014, 12:05 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26430/
> ---
> 
> (Updated Oct. 8, 2014, 12:05 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman.
> 
> 
> Bugs: AURORA-333
> https://issues.apache.org/jira/browse/AURORA-333
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This removes several long deprecated configuration options from aurora.
> 
> The features removed are cron_policy, daemon, health_check_interval_secs, 
> recipes and the PackerObject
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
>   src/main/python/apache/aurora/client/binding_helper.py 
> 6d6a06785c6840e4345e304eb4e242682676ac66 
>   src/main/python/apache/aurora/client/config.py 
> e440f587d100ce46b2df85ccc663912c615051ef 
>   src/main/python/apache/aurora/config/recipes.py 
> 68b5d252f87a592d4c2f7d52525163829bea2cc9 
>   src/main/python/apache/aurora/config/schema/base.py 
> f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
>   src/main/python/apache/aurora/config/thrift.py 
> 288fb40f65629c8fd4eb7d92c8bf02369237de3b 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 38ffdb86c5f577ebf3a482128588331a63af15d1 
>   src/test/python/apache/aurora/client/cli/util.py 
> ff7eda20dbba073c8b24fbe3f4389292aab2d128 
>   src/test/python/apache/aurora/client/commands/util.py 
> 663f2f4a16113a36826943b7238cad900ae0dcd2 
>   src/test/python/apache/aurora/config/test_thrift.py 
> fd28313df2cfd5a9c7d00f6d329518b4caabacb2 
> 
> Diff: https://reviews.apache.org/r/26430/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26425: Fixing quota checking for updates.

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26425/#review55865
---


I'm underwater on reviews and unable to review this promptly - please remove me 
from the People line.

- Kevin Sweeney


On Oct. 7, 2014, 3:03 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26425/
> ---
> 
> (Updated Oct. 7, 2014, 3:03 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-802
> https://issues.apache.org/jira/browse/AURORA-802
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Rolling up prod consumption does not work for quota checking during job 
> update intitiation where per-instance filtering is required. Splitting the 
> QuotaManager interface into 2 use cases:
> - createJob/addInstances
> - startJobUpdate
> 
> Reverting the IResourceAggregate abstraction in QuotaManager in favor of 
> task/update objects to simplify handling.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 2442b06f318c8d0cefe60296950baa1b916b42f7 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java 
> d197dd515eb646cb4babadf22e9cf6480a919060 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5dcae4a6132026504cf02093ad4c68ab521e4ab8 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b58c8f363e6e7c72accaf590b2a7cb7bf24275ea 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 
> 
> Diff: https://reviews.apache.org/r/26425/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> Also, tested various update scenarios in vagrant.
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25835/#review55864
---


Patch fails to apply:

% ./rbt patch -c 25835
Patch is being applied from request 25835 with diff revision  4.
Failed to execute command: ['git', 'commit', '-m', u"Serve HTTP assets out of a 
standard classpath root.\n\nTesting Done:\n./gradlew build -Pq\n\nConfirmed 
everything works in scheduler UI in vagrant image.\n\nOne thing I've seen 
locally is lost tasks in vagrant, but I *think*\nthat's just due to my image 
being somehow misconfigured, and not\nthese changes. I'm going to destroy and 
rebuild the image to see if\nthat cleans things up.\n\nBugs closed: 
AURORA-678\n\nReviewed at https://reviews.apache.org/r/25835/\n";, 
u'--author="Joshua Cohen "']
Performing Python import order check.
SUCCESS
Performing Python checkstyle.
SUCCESS
On branch master

- Kevin Sweeney


On Sept. 19, 2014, 2:39 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25835/
> ---
> 
> (Updated Sept. 19, 2014, 2:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-678
> https://issues.apache.org/jira/browse/AURORA-678
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Serve HTTP assets out of a standard claspath root.
> 
> There's a lot of moved files in this diff, so apologies for it being 
> essentially unreadable. ReviewBoard actually seems to choke about half the 
> time just loading the individual pages.
> 
> For the sake of convenience/sanity, the actual meat of the changes can be 
> found in:
> 
> build.gradle: https://reviews.apache.org/r/25835/diff/#336
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: 
> https://reviews.apache.org/r/25835/diff/1/?page=17#338
> src/main/resources/scheduler/assets/index.html: 
> https://reviews.apache.org/r/25835/diff/1/?page=18#363
> 
> Everything else is just a rename/move.
> 
> 
> Diffs
> -
> 
>   3rdparty/javascript/bower_components/angular-bootstrap/.bower.json  
>   3rdparty/javascript/bower_components/angular-bootstrap/bower.json  
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 
>  
>   
> 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js
>   
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js  
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js  
>   3rdparty/javascript/bower_components/angular-route/.bower.json  
>   3rdparty/javascript/bower_components/angular-route/README.md  
>   3rdparty/javascript/bower_components/angular-route/angular-route.js  
>   3rdparty/javascript/bower_components/angular-route/angular-route.min.js  
>   3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 
>  
>   3rdparty/javascript/bower_components/angular-route/bower.json  
>   3rdparty/javascript/bower_components/angular/.bower.json  
>   3rdparty/javascript/bower_components/angular/README.md  
>   3rdparty/javascript/bower_components/angular/angular-csp.css  
>   3rdparty/javascript/bower_components/angular/angular.js  
>   3rdparty/javascript/bower_components/angular/angular.min.js  
>   3rdparty/javascript/bower_components/angular/angular.min.js.gzip  
>   3rdparty/javascript/bower_components/angular/angular.min.js.map  
>   3rdparty/javascript/bower_components/angular/bower.json  
>   3rdparty/javascript/bower_components/bootstrap/.bower.json  
>   3rdparty/javascript/bower_components/bootstrap/Gruntfile.js  
>   3rdparty/javascript/bower_components/bootstrap/LICENSE  
>   3rdparty/javascript/bower_components/bootstrap/README.md  
>   3rdparty/javascript/bower_components/bootstrap/bower.json  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 
>  
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css
>   
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css  
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff
>   
>   3rdparty/javascript/bower_components/boots

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25835/#review55862
---


Patch fails to apply:

% ./rbt patch -c 25835
Patch is being applied from request 25835 with diff revision  4.
Failed to execute command: ['git', 'commit', '-m', u"Serve HTTP assets out of a 
standard classpath root.\n\nTesting Done:\n./gradlew build -Pq\n\nConfirmed 
everything works in scheduler UI in vagrant image.\n\nOne thing I've seen 
locally is lost tasks in vagrant, but I *think*\nthat's just due to my image 
being somehow misconfigured, and not\nthese changes. I'm going to destroy and 
rebuild the image to see if\nthat cleans things up.\n\nBugs closed: 
AURORA-678\n\nReviewed at https://reviews.apache.org/r/25835/\n";, 
u'--author="Joshua Cohen "']
Performing Python import order check.
SUCCESS
Performing Python checkstyle.
SUCCESS
On branch master

- Kevin Sweeney


On Sept. 19, 2014, 2:39 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25835/
> ---
> 
> (Updated Sept. 19, 2014, 2:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-678
> https://issues.apache.org/jira/browse/AURORA-678
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Serve HTTP assets out of a standard claspath root.
> 
> There's a lot of moved files in this diff, so apologies for it being 
> essentially unreadable. ReviewBoard actually seems to choke about half the 
> time just loading the individual pages.
> 
> For the sake of convenience/sanity, the actual meat of the changes can be 
> found in:
> 
> build.gradle: https://reviews.apache.org/r/25835/diff/#336
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: 
> https://reviews.apache.org/r/25835/diff/1/?page=17#338
> src/main/resources/scheduler/assets/index.html: 
> https://reviews.apache.org/r/25835/diff/1/?page=18#363
> 
> Everything else is just a rename/move.
> 
> 
> Diffs
> -
> 
>   3rdparty/javascript/bower_components/angular-bootstrap/.bower.json  
>   3rdparty/javascript/bower_components/angular-bootstrap/bower.json  
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 
>  
>   
> 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js
>   
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js  
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js  
>   3rdparty/javascript/bower_components/angular-route/.bower.json  
>   3rdparty/javascript/bower_components/angular-route/README.md  
>   3rdparty/javascript/bower_components/angular-route/angular-route.js  
>   3rdparty/javascript/bower_components/angular-route/angular-route.min.js  
>   3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 
>  
>   3rdparty/javascript/bower_components/angular-route/bower.json  
>   3rdparty/javascript/bower_components/angular/.bower.json  
>   3rdparty/javascript/bower_components/angular/README.md  
>   3rdparty/javascript/bower_components/angular/angular-csp.css  
>   3rdparty/javascript/bower_components/angular/angular.js  
>   3rdparty/javascript/bower_components/angular/angular.min.js  
>   3rdparty/javascript/bower_components/angular/angular.min.js.gzip  
>   3rdparty/javascript/bower_components/angular/angular.min.js.map  
>   3rdparty/javascript/bower_components/angular/bower.json  
>   3rdparty/javascript/bower_components/bootstrap/.bower.json  
>   3rdparty/javascript/bower_components/bootstrap/Gruntfile.js  
>   3rdparty/javascript/bower_components/bootstrap/LICENSE  
>   3rdparty/javascript/bower_components/bootstrap/README.md  
>   3rdparty/javascript/bower_components/bootstrap/bower.json  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 
>  
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css
>   
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css  
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff
>   
>   3rdparty/javascript/bower_components/boots

Re: Review Request 26428: Fixing python style violations.

[Maxim Khutornenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/
---

(Updated Oct. 8, 2014, 7:22 p.m.)


Review request for Aurora and Brian Wickman.


Changes
---

Fixing more violations.


Repository: aurora


Description
---

Pre-commit hook was no longer working properly. Added target to match jenkins 
definition.


Diffs (updated)
-

  build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c 
  src/main/python/apache/aurora/executor/gc_executor.py 
a11feb910cd116f53c9949343dfe7ecbd17d793c 
  src/test/python/apache/aurora/client/api/test_updater.py 
fc6a057c6c650d9ac9800b009e544dfad0c809bf 
  src/test/python/apache/aurora/client/cli/test_create.py 
88d1b350b09012684bb4f7263657cdff083df215 
  src/test/python/apache/aurora/client/cli/test_inspect.py 
b64fbf9864693d8758baf01ba2293df525adebb7 
  src/test/python/apache/aurora/client/cli/test_status.py 
38ffdb86c5f577ebf3a482128588331a63af15d1 
  src/test/python/apache/aurora/client/cli/util.py 
4e9b6362728ecfd6e8a6efbc433f63f42aef60ad 

Diff: https://reviews.apache.org/r/26428/diff/


Testing
---

./build-support/hooks/pre-commit 
Performing Python import order check.
SUCCESS
Performing Python checkstyle.
SUCCESS

$ build-support/python/checkstyle-check src


Thanks,

Maxim Khutornenko

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26308/
---

(Updated Oct. 8, 2014, 7:19 p.m.)


Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.


Changes
---

Addressed merge conflicts on master, fixed python style errors that snuck in 
since the last diff.


Repository: aurora


Description
---

Fixes two problems:

- mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
objects were being passed around and somehow resulted in the test case passing.
- use of `threading.Event()` was broken in scheduler_client.py.  I don't think 
it's possible to enter those branches.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/__init__.py 
26300792594e4005dacc139a9f89711b8a66ab61 
  src/main/python/apache/aurora/client/api/command_runner.py 
a1fed5fc75dde3a79c840515e6daa4741156ef97 
  src/main/python/apache/aurora/client/api/job_monitor.py 
18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
311c954f1db245b75192d00c6aca0721085fbf32 
  src/main/python/apache/aurora/client/api/updater.py 
bb4a7555851341e450da408441975f078f2b5576 
  src/main/python/apache/aurora/common/aurora_job_key.py 
a7ca7b6df6b9566b3ce617283ccac948deb2eb83 
  src/test/python/apache/aurora/client/api/test_job_monitor.py 
5b26539f86a0a82f72753a803a769eda77cbc332 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1835843f1795b0530874ec561582df17acfbce65 
  src/test/python/apache/aurora/client/api/test_updater.py 
fc6a057c6c650d9ac9800b009e544dfad0c809bf 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
733327b557df3e81c6d723bf24e753442fa375ca 
  src/test/python/apache/aurora/client/cli/test_cancel_update.py 
c15e142930c9474c7873dd931261b6ab4eb5967f 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 
  src/test/python/apache/aurora/client/cli/test_create.py 
88d1b350b09012684bb4f7263657cdff083df215 
  src/test/python/apache/aurora/client/cli/test_diff.py 
e1a6f764830e06c73d0bc10a3b5da67219da836c 
  src/test/python/apache/aurora/client/cli/test_inspect.py 
b64fbf9864693d8758baf01ba2293df525adebb7 
  src/test/python/apache/aurora/client/cli/test_kill.py 
e3a366bf67074e50787394cad58d5e01359b641e 
  src/test/python/apache/aurora/client/cli/test_logging.py 
6285fbb07442291c2dc4096e68eb285c98994097 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
7c79c7a75e77e47544684c0f767d248d2540901a 
  src/test/python/apache/aurora/client/cli/test_status.py 
38ffdb86c5f577ebf3a482128588331a63af15d1 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
1ce9a632874e818eee71573cd481842affae3615 
  src/test/python/apache/aurora/client/cli/test_update.py 
85b1db19d89967a741bfba7964eeb368426f0b61 
  src/test/python/apache/aurora/client/cli/util.py 
4e9b6362728ecfd6e8a6efbc433f63f42aef60ad 
  src/test/python/apache/aurora/client/commands/test_admin.py 
1192556c027dc3adf16bb37adeac7798cf9ef93d 
  src/test/python/apache/aurora/client/commands/test_cancel_update.py 
5f05ef7c0643d189de3de38c75aae58c2a3814a4 
  src/test/python/apache/aurora/client/commands/test_create.py 
7503345ea1c0f224a894ce02cc2c2d8719574e32 
  src/test/python/apache/aurora/client/commands/test_diff.py 
8f5da7d2bca9b0486b635afe49d3885151624e12 
  src/test/python/apache/aurora/client/commands/test_hooks.py 
0861f13b13a8406950ba953efba0ffae186a8253 
  src/test/python/apache/aurora/client/commands/test_kill.py 
c0a6fd44c5691cde50746ffdec325bb11a33469a 
  src/test/python/apache/aurora/client/commands/test_run.py 
e97b5156609f80a4170028e780bcd891e56983ff 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
  src/test/python/apache/aurora/client/commands/test_status.py 
bda1f28d544ae48428129f8167d8632ef27f5fba 
  src/test/python/apache/aurora/client/commands/test_update.py 
af2cbc7f88287201a472ba36902b00d90bc77d3b 

Diff: https://reviews.apache.org/r/26308/diff/


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26424: Disable requests http connection logging.

[Joe Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/#review55860
---

Ship it!


Inlining inside `__init__` WFM. thanks!

- Joe Smith


On Oct. 8, 2014, 9:54 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26424/
> ---
> 
> (Updated Oct. 8, 2014, 9:54 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Bill Farner.
> 
> 
> Bugs: AURORA-770
> https://issues.apache.org/jira/browse/AURORA-770
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> .
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 6f7c355d725b5e537cc4ae471170eaa8431da326 
>   src/test/python/apache/aurora/common/test_transport.py 
> c722eae2d04dec90e9c772f49c578184a2bdf76c 
> 
> Diff: https://reviews.apache.org/r/26424/diff/
> 
> 
> Testing
> ---
> 
> Added hokey test, ran hokey test.
> 
> Also verified the lack of http connection logs when running client commands 
> directly.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py

[Joe Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26431/#review55859
---



src/test/python/apache/aurora/client/commands/test_maintenance.py


can this be a mock that we assert gets called 3 times?

(This test is a little bit of an integration test since it goes in and is 
also testing perform_maintenace, but a full refactor of this test may be too 
much for now)


- Joe Smith


On Oct. 7, 2014, 5:13 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26431/
> ---
> 
> (Updated Oct. 7, 2014, 5:13 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-806
> https://issues.apache.org/jira/browse/AURORA-806
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moving post_drain script functionality into host_maintenance.py to support 
> per batch execution.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 9c2a9f77109791da574e1624d27b6b7096a2678e 
>   src/main/python/apache/aurora/client/commands/maintenance.py 
> e465d973e9f764076e18491e1691d44303c0f388 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> 40228df59e43bc6034f2dc651c166a0c4b78aea8 
>   src/test/python/apache/aurora/client/commands/test_maintenance.py 
> d86aaf677804301fa5ddf1f76dba552f4fafb8c3 
> 
> Diff: https://reviews.apache.org/r/26431/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Joe Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26308/#review55858
---

Ship it!


Ship It!

- Joe Smith


On Oct. 4, 2014, 10:55 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26308/
> ---
> 
> (Updated Oct. 4, 2014, 10:55 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes two problems:
> 
> - mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
> objects were being passed around and somehow resulted in the test case 
> passing.
> - use of `threading.Event()` was broken in scheduler_client.py.  I don't 
> think it's possible to enter those branches.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 26300792594e4005dacc139a9f89711b8a66ab61 
>   src/main/python/apache/aurora/client/api/command_runner.py 
> a1fed5fc75dde3a79c840515e6daa4741156ef97 
>   src/main/python/apache/aurora/client/api/job_monitor.py 
> 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 311c954f1db245b75192d00c6aca0721085fbf32 
>   src/main/python/apache/aurora/client/api/updater.py 
> bf608981c2f2e7960b68c3fbda144277a59a3d40 
>   src/main/python/apache/aurora/common/aurora_job_key.py 
> a7ca7b6df6b9566b3ce617283ccac948deb2eb83 
>   src/test/python/apache/aurora/client/api/test_job_monitor.py 
> 5b26539f86a0a82f72753a803a769eda77cbc332 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1835843f1795b0530874ec561582df17acfbce65 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 6905831b23a84320e7f41843efd62b86da366c0b 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_cancel_update.py 
> c15e142930c9474c7873dd931261b6ab4eb5967f 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> e1a6f764830e06c73d0bc10a3b5da67219da836c 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> e3a366bf67074e50787394cad58d5e01359b641e 
>   src/test/python/apache/aurora/client/cli/test_logging.py 
> 6285fbb07442291c2dc4096e68eb285c98994097 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 1ce9a632874e818eee71573cd481842affae3615 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 85b1db19d89967a741bfba7964eeb368426f0b61 
>   src/test/python/apache/aurora/client/commands/test_admin.py 
> 1192556c027dc3adf16bb37adeac7798cf9ef93d 
>   src/test/python/apache/aurora/client/commands/test_cancel_update.py 
> 5f05ef7c0643d189de3de38c75aae58c2a3814a4 
>   src/test/python/apache/aurora/client/commands/test_create.py 
> 7503345ea1c0f224a894ce02cc2c2d8719574e32 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> 8f5da7d2bca9b0486b635afe49d3885151624e12 
>   src/test/python/apache/aurora/client/commands/test_hooks.py 
> 0861f13b13a8406950ba953efba0ffae186a8253 
>   src/test/python/apache/aurora/client/commands/test_kill.py 
> c0a6fd44c5691cde50746ffdec325bb11a33469a 
>   src/test/python/apache/aurora/client/commands/test_run.py 
> e97b5156609f80a4170028e780bcd891e56983ff 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> bda1f28d544ae48428129f8167d8632ef27f5fba 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> af2cbc7f88287201a472ba36902b00d90bc77d3b 
> 
> Diff: https://reviews.apache.org/r/26308/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26428: Fixing python style violations.

[Brian Wickman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/#review55857
---

Ship it!


Ship It!

- Brian Wickman


On Oct. 8, 2014, 12:17 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26428/
> ---
> 
> (Updated Oct. 8, 2014, 12:17 a.m.)
> 
> 
> Review request for Aurora and Brian Wickman.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Pre-commit hook was no longer working properly. Added target to match jenkins 
> definition.
> 
> 
> Diffs
> -
> 
>   build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> fc6a057c6c650d9ac9800b009e544dfad0c809bf 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 38ffdb86c5f577ebf3a482128588331a63af15d1 
>   src/test/python/apache/aurora/client/cli/util.py 
> ff7eda20dbba073c8b24fbe3f4389292aab2d128 
> 
> Diff: https://reviews.apache.org/r/26428/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/hooks/pre-commit 
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> SUCCESS
> 
> $ build-support/python/checkstyle-check src
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26428: Fixing python style violations.

[Brian Wickman]

> On Oct. 7, 2014, 11:31 p.m., Brian Wickman wrote:
> > build-support/hooks/pre-commit, line 41
> > 
> >
> > the reason i didn't add this to the pre-commit is beause the default 
> > behavior for the pre-commit hook is to only check the diff against master.  
> > it's possible that it needs more context e.g. +-2 lines in the file 
> > beyond the diff, so it might miss stuff.
> > 
> > doing against source for every commit might be too time consuming.  out 
> > of curiosity, how long does it take to run this check on your laptop?
> 
> Maxim Khutornenko wrote:
> It's between 15-20 seconds now, which is much slower than usual but I'd 
> rather make it slower and useful or drop completely and rely on jenkins 
> script. Your call.

We should either fix --diff or change its behavior to be more conservative by 
performing checkstyle on the entire file if it is involved in the diff.  In the 
meantime, I will give a ship, since pre-commits are still opt-in.


- Brian


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/#review55729
---


On Oct. 8, 2014, 12:17 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26428/
> ---
> 
> (Updated Oct. 8, 2014, 12:17 a.m.)
> 
> 
> Review request for Aurora and Brian Wickman.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Pre-commit hook was no longer working properly. Added target to match jenkins 
> definition.
> 
> 
> Diffs
> -
> 
>   build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> fc6a057c6c650d9ac9800b009e544dfad0c809bf 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 38ffdb86c5f577ebf3a482128588331a63af15d1 
>   src/test/python/apache/aurora/client/cli/util.py 
> ff7eda20dbba073c8b24fbe3f4389292aab2d128 
> 
> Diff: https://reviews.apache.org/r/26428/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/hooks/pre-commit 
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> SUCCESS
> 
> $ build-support/python/checkstyle-check src
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26300: Don't kill GC Executor after period of inactivity

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26300/
---

(Updated Oct. 8, 2014, 11:56 a.m.)


Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman.


Changes
---

rebase


Bugs: AURORA-788
https://issues.apache.org/jira/browse/AURORA-788


Repository: aurora


Description
---

The GC executor is configured to exit after 15 minutes of inactivity. This 
leads to a race where the mesos slave gets a launchTask message for a GC 
executor just as the executor has exited, causing TASK_LOST noise. This also 
increases the risk that a slave will lose its GC executor due to the scheduler 
not being able to find a slot for it (since GC executors will have a higher 
churn rate).

Cluster operators will still be able to deploy new versions of the GC executor 
as the 24-hour max lifetime limit is still in place. This patch only removes 
the inactivity limit.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/gc_executor.py 
788671e32d2f995b954e906d8866019ed66c970d 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
1905fe35de31e667f499f7945952f04dc13aac06 

Diff: https://reviews.apache.org/r/26300/diff/


Testing
---

./pants src/test/python/apache/aurora/executor:gc_executor


Thanks,

Kevin Sweeney

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/#review55854
---

Ship it!


Thanks for the extra look!

- Bill Farner


On Oct. 8, 2014, 5:27 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26422/
> ---
> 
> (Updated Oct. 8, 2014, 5:27 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-801
> https://issues.apache.org/jira/browse/AURORA-801
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Drop syncrhonized from JobUpdateEventSubscriber
> 
> This fixes a startup deadlock.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java
>  49d8b7a6c4adc4c58049c439bd09019c9e6885b1 
> 
> Diff: https://reviews.apache.org/r/26422/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Manually verified that all delegated calls to the JobUpdateController are 
> already protected by the storage write-lock.
> 
> Rather than add a potentially-flaky regression test (like the one added in 
> https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
> deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[Kevin Sweeney]

> On Oct. 8, 2014, 9:19 a.m., Bill Farner wrote:
> > While our minds are on deadlock risks, it's a good idea to assess other 
> > potential vulnerabilities.
> > 
> > A quick filter to find other potential sources deserving a glance:
> > $ grep -Rl synchronized src/main/java | xargs grep -l Storage
> > src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
> > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
> > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
> > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
> > src/main/java/org/apache/aurora/scheduler/TaskVars.java
> > 
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
> 
> Kevin Sweeney wrote:
> My proposal is to add runtime deadlock detection for these cases via 
> CycleDetectingLockFactory. I have runtime evidence that this deadlock exists 
> and would like to keep this change small in scope. Happy to add this as a 
> followup item to AURORA-800.
> 
> Bill Farner wrote:
> That effort shouldn't cause us to skip due diligence of a skim for other 
> places we're vulnerable.

A cursory look through doesn't reveal any immediate concerns. Preemptor does 
acquire the storage lock in a synchronized method; however the only caller of 
Preemptor always holds the storage write lock. Others just use synchronization 
to ensure consistent internal state.

Note I used 'synchronized ' to avoid synchronizedMap.
% grep -Rl 'synchronized '  src/main/java | xargs grep -lE 
'(.write|.consistentRead|.consistentFetchTasks)'
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
src/main/java/org/apache/aurora/scheduler/TaskVars.java
src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLog.java

Of course, this doesn't reveal cases where a call to a dependency might cause 
the storage lock to be acquired, nor does it protect against accidental 
introduction of new deadlocks so AURORA-800 is still relevant.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/#review55803
---


On Oct. 8, 2014, 10:27 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26422/
> ---
> 
> (Updated Oct. 8, 2014, 10:27 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-801
> https://issues.apache.org/jira/browse/AURORA-801
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Drop syncrhonized from JobUpdateEventSubscriber
> 
> This fixes a startup deadlock.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java
>  49d8b7a6c4adc4c58049c439bd09019c9e6885b1 
> 
> Diff: https://reviews.apache.org/r/26422/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Manually verified that all delegated calls to the JobUpdateController are 
> already protected by the storage write-lock.
> 
> Rather than add a potentially-flaky regression test (like the one added in 
> https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
> deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26320: Skip checkstyle on python file in 3rdparty.

[Joshua Cohen]

> On Oct. 8, 2014, 6:02 p.m., Kevin Sweeney wrote:
> > Apologies for the review delay.
> > 
> > Is there a way we can tell checkstyle to avoid this file at all? I'd rather 
> > not make it a policy to patch each individual bower component we checkin.

Not without patching checkstyle alas.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26320/#review55847
---


On Oct. 3, 2014, 4:51 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26320/
> ---
> 
> (Updated Oct. 3, 2014, 4:51 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-780
> https://issues.apache.org/jira/browse/AURORA-780
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Skip checkstyle on python file in 3rdparty.
> 
> 
> Diffs
> -
> 
>   3rdparty/javascript/bower_components/bootstrap/test-infra/s3_cache.py 
> 472963a1e4a6c9ace6044273c7f728812aa8458b 
> 
> Diff: https://reviews.apache.org/r/26320/diff/
> 
> 
> Testing
> ---
> 
> Committed file w/ precommit hook in place.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26445: Fix error in incorrect deprecation warning on v1 "ssh" command.

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26445/#review55849
---

Ship it!


Ship It!

- David McLaughlin


On Oct. 8, 2014, 3:58 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26445/
> ---
> 
> (Updated Oct. 8, 2014, 3:58 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-804
> https://issues.apache.org/jira/browse/aurora-804
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The error message incorrectly displayed a list-value for the
> "--tunnels" parameter, when in fact it should expand the list
> into multiple instances of "--tunnels".
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> 9ee94a0bdfd8bbc030ae978e2ac3fe39571b4ce2 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
> 
> Diff: https://reviews.apache.org/r/26445/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26320: Skip checkstyle on python file in 3rdparty.

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26320/#review55847
---


Apologies for the review delay.

Is there a way we can tell checkstyle to avoid this file at all? I'd rather not 
make it a policy to patch each individual bower component we checkin.

- Kevin Sweeney


On Oct. 3, 2014, 9:51 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26320/
> ---
> 
> (Updated Oct. 3, 2014, 9:51 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-780
> https://issues.apache.org/jira/browse/AURORA-780
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Skip checkstyle on python file in 3rdparty.
> 
> 
> Diffs
> -
> 
>   3rdparty/javascript/bower_components/bootstrap/test-infra/s3_cache.py 
> 472963a1e4a6c9ace6044273c7f728812aa8458b 
> 
> Diff: https://reviews.apache.org/r/26320/diff/
> 
> 
> Testing
> ---
> 
> Committed file w/ precommit hook in place.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26308/#review55846
---

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 4, 2014, 10:55 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26308/
> ---
> 
> (Updated Oct. 4, 2014, 10:55 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes two problems:
> 
> - mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
> objects were being passed around and somehow resulted in the test case 
> passing.
> - use of `threading.Event()` was broken in scheduler_client.py.  I don't 
> think it's possible to enter those branches.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 26300792594e4005dacc139a9f89711b8a66ab61 
>   src/main/python/apache/aurora/client/api/command_runner.py 
> a1fed5fc75dde3a79c840515e6daa4741156ef97 
>   src/main/python/apache/aurora/client/api/job_monitor.py 
> 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 311c954f1db245b75192d00c6aca0721085fbf32 
>   src/main/python/apache/aurora/client/api/updater.py 
> bf608981c2f2e7960b68c3fbda144277a59a3d40 
>   src/main/python/apache/aurora/common/aurora_job_key.py 
> a7ca7b6df6b9566b3ce617283ccac948deb2eb83 
>   src/test/python/apache/aurora/client/api/test_job_monitor.py 
> 5b26539f86a0a82f72753a803a769eda77cbc332 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1835843f1795b0530874ec561582df17acfbce65 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 6905831b23a84320e7f41843efd62b86da366c0b 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_cancel_update.py 
> c15e142930c9474c7873dd931261b6ab4eb5967f 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> e1a6f764830e06c73d0bc10a3b5da67219da836c 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> e3a366bf67074e50787394cad58d5e01359b641e 
>   src/test/python/apache/aurora/client/cli/test_logging.py 
> 6285fbb07442291c2dc4096e68eb285c98994097 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 1ce9a632874e818eee71573cd481842affae3615 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 85b1db19d89967a741bfba7964eeb368426f0b61 
>   src/test/python/apache/aurora/client/commands/test_admin.py 
> 1192556c027dc3adf16bb37adeac7798cf9ef93d 
>   src/test/python/apache/aurora/client/commands/test_cancel_update.py 
> 5f05ef7c0643d189de3de38c75aae58c2a3814a4 
>   src/test/python/apache/aurora/client/commands/test_create.py 
> 7503345ea1c0f224a894ce02cc2c2d8719574e32 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> 8f5da7d2bca9b0486b635afe49d3885151624e12 
>   src/test/python/apache/aurora/client/commands/test_hooks.py 
> 0861f13b13a8406950ba953efba0ffae186a8253 
>   src/test/python/apache/aurora/client/commands/test_kill.py 
> c0a6fd44c5691cde50746ffdec325bb11a33469a 
>   src/test/python/apache/aurora/client/commands/test_run.py 
> e97b5156609f80a4170028e780bcd891e56983ff 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> bda1f28d544ae48428129f8167d8632ef27f5fba 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> af2cbc7f88287201a472ba36902b00d90bc77d3b 
> 
> Diff: https://reviews.apache.org/r/26308/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[Bill Farner]

> On Oct. 8, 2014, 4:19 p.m., Bill Farner wrote:
> > While our minds are on deadlock risks, it's a good idea to assess other 
> > potential vulnerabilities.
> > 
> > A quick filter to find other potential sources deserving a glance:
> > $ grep -Rl synchronized src/main/java | xargs grep -l Storage
> > src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
> > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
> > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
> > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
> > src/main/java/org/apache/aurora/scheduler/TaskVars.java
> > 
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
> 
> Kevin Sweeney wrote:
> My proposal is to add runtime deadlock detection for these cases via 
> CycleDetectingLockFactory. I have runtime evidence that this deadlock exists 
> and would like to keep this change small in scope. Happy to add this as a 
> followup item to AURORA-800.

That effort shouldn't cause us to skip due diligence of a skim for other places 
we're vulnerable.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/#review55803
---


On Oct. 8, 2014, 5:27 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26422/
> ---
> 
> (Updated Oct. 8, 2014, 5:27 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-801
> https://issues.apache.org/jira/browse/AURORA-801
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Drop syncrhonized from JobUpdateEventSubscriber
> 
> This fixes a startup deadlock.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java
>  49d8b7a6c4adc4c58049c439bd09019c9e6885b1 
> 
> Diff: https://reviews.apache.org/r/26422/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Manually verified that all delegated calls to the JobUpdateController are 
> already protected by the storage write-lock.
> 
> Rather than add a potentially-flaky regression test (like the one added in 
> https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
> deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

[Bill Farner]

> On Oct. 3, 2014, 4:43 p.m., Bill Farner wrote:
> > Anything left to do here before Kevin commits?
> 
> Joshua Cohen wrote:
> Nope, he tried to commit but was blocked by checkstyle flagging 3rdparty 
> python (https://issues.apache.org/jira/browse/AURORA-780). I sent a review 
> out to address that this morning, so once that ships he should be able to 
> merge this.

Kevin - anything else blocking this?


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25835/#review55353
---


On Sept. 19, 2014, 9:39 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25835/
> ---
> 
> (Updated Sept. 19, 2014, 9:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-678
> https://issues.apache.org/jira/browse/AURORA-678
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Serve HTTP assets out of a standard claspath root.
> 
> There's a lot of moved files in this diff, so apologies for it being 
> essentially unreadable. ReviewBoard actually seems to choke about half the 
> time just loading the individual pages.
> 
> For the sake of convenience/sanity, the actual meat of the changes can be 
> found in:
> 
> build.gradle: https://reviews.apache.org/r/25835/diff/#336
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: 
> https://reviews.apache.org/r/25835/diff/1/?page=17#338
> src/main/resources/scheduler/assets/index.html: 
> https://reviews.apache.org/r/25835/diff/1/?page=18#363
> 
> Everything else is just a rename/move.
> 
> 
> Diffs
> -
> 
>   3rdparty/javascript/bower_components/angular-bootstrap/.bower.json  
>   3rdparty/javascript/bower_components/angular-bootstrap/bower.json  
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 
>  
>   
> 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js
>   
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js  
>   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js  
>   3rdparty/javascript/bower_components/angular-route/.bower.json  
>   3rdparty/javascript/bower_components/angular-route/README.md  
>   3rdparty/javascript/bower_components/angular-route/angular-route.js  
>   3rdparty/javascript/bower_components/angular-route/angular-route.min.js  
>   3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 
>  
>   3rdparty/javascript/bower_components/angular-route/bower.json  
>   3rdparty/javascript/bower_components/angular/.bower.json  
>   3rdparty/javascript/bower_components/angular/README.md  
>   3rdparty/javascript/bower_components/angular/angular-csp.css  
>   3rdparty/javascript/bower_components/angular/angular.js  
>   3rdparty/javascript/bower_components/angular/angular.min.js  
>   3rdparty/javascript/bower_components/angular/angular.min.js.gzip  
>   3rdparty/javascript/bower_components/angular/angular.min.js.map  
>   3rdparty/javascript/bower_components/angular/bower.json  
>   3rdparty/javascript/bower_components/bootstrap/.bower.json  
>   3rdparty/javascript/bower_components/bootstrap/Gruntfile.js  
>   3rdparty/javascript/bower_components/bootstrap/LICENSE  
>   3rdparty/javascript/bower_components/bootstrap/README.md  
>   3rdparty/javascript/bower_components/bootstrap/bower.json  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 
>  
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css
>   
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map  
>   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css  
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff
>   
>   3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js  
>   3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js  
>   
> 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.eot
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.svg
>   
>   
> 3rdparty/javascript/bower_components/bootstrap/fonts/g

Re: Review Request 26432: Reject new GC tasks when shutting down

[Joe Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26432/#review55839
---

Ship it!


Awesome

- Joe Smith


On Oct. 8, 2014, 10:28 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26432/
> ---
> 
> (Updated Oct. 8, 2014, 10:28 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-807
> https://issues.apache.org/jira/browse/AURORA-807
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Reject new GC tasks when shutting down
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> d90dc68ec1526110a484cfc2b6c4658abdc2e871 
> 
> Diff: https://reviews.apache.org/r/26432/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26445: Fix error in incorrect deprecation warning on v1 "ssh" command.

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26445/#review55840
---

Ship it!


Ship It!

- Zameer Manji


On Oct. 8, 2014, 8:58 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26445/
> ---
> 
> (Updated Oct. 8, 2014, 8:58 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-804
> https://issues.apache.org/jira/browse/aurora-804
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The error message incorrectly displayed a list-value for the
> "--tunnels" parameter, when in fact it should expand the list
> into multiple instances of "--tunnels".
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> 9ee94a0bdfd8bbc030ae978e2ac3fe39571b4ce2 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
> 
> Diff: https://reviews.apache.org/r/26445/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26448: Move the error-log seed file to a user specified directory.

[Zameer Manji]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26448/#review55838
---

Ship it!


Ship It!

- Zameer Manji


On Oct. 8, 2014, 10:32 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26448/
> ---
> 
> (Updated Oct. 8, 2014, 10:32 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-779
> https://issues.apache.org/jira/browse/aurora-779
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Instead of dumping error traces into the users current directory,
> dump them into a user specified directory, with a benign default.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 5c0688f5ea184e2c0b1f8e1b54a109d8260a12fa 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 4fdcf6df493f63aae672e0834214dad208cf4110 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8dc0ccb78539f8b0c7829ec2927da93d4afa9ada 
>   src/test/python/apache/aurora/client/cli/test_inspect.py 
> fd35fe0185c1850910bd18d6a1e5831cd3effa6f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
>   src/test/python/apache/aurora/client/cli/util.py 
> ff7eda20dbba073c8b24fbe3f4389292aab2d128 
> 
> Diff: https://reviews.apache.org/r/26448/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test; verified all tests pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26448: Move the error-log seed file to a user specified directory.

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26448/#review55837
---

Ship it!


Ship It!

- David McLaughlin


On Oct. 8, 2014, 5:32 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26448/
> ---
> 
> (Updated Oct. 8, 2014, 5:32 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-779
> https://issues.apache.org/jira/browse/aurora-779
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Instead of dumping error traces into the users current directory,
> dump them into a user specified directory, with a benign default.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 5c0688f5ea184e2c0b1f8e1b54a109d8260a12fa 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 4fdcf6df493f63aae672e0834214dad208cf4110 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8dc0ccb78539f8b0c7829ec2927da93d4afa9ada 
>   src/test/python/apache/aurora/client/cli/test_inspect.py 
> fd35fe0185c1850910bd18d6a1e5831cd3effa6f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
>   src/test/python/apache/aurora/client/cli/util.py 
> ff7eda20dbba073c8b24fbe3f4389292aab2d128 
> 
> Diff: https://reviews.apache.org/r/26448/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test; verified all tests pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.

[Mark Chu-Carroll]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26363/
---

(Updated Oct. 8, 2014, 1:40 p.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

Remove debug prints.


Bugs: aurora-792
https://issues.apache.org/jira/browse/aurora-792


Repository: aurora


Description
---

Make the large-update check in the client update command consider instance 
parameters.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/context.py 
301531fcb443297facb78d87a18073c8b7fd4064 
  src/main/python/apache/aurora/client/cli/jobs.py 
103fb53cb5101d3e025d8712e3887bccdfbb6aeb 
  src/test/python/apache/aurora/client/cli/BUILD 
8ce6bd5b7faa1579372fb6935180ea982af64af8 
  src/test/python/apache/aurora/client/cli/test_update.py 
85b1db19d89967a741bfba7964eeb368426f0b61 

Diff: https://reviews.apache.org/r/26363/diff/


Testing
---

New unit tests added, all test pass.


Thanks,

Mark Chu-Carroll

Re: Review Request 26432: Reject new GC tasks when shutting down

[Brian Wickman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26432/#review55836
---

Ship it!


Ship It!

- Brian Wickman


On Oct. 8, 2014, 5:28 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26432/
> ---
> 
> (Updated Oct. 8, 2014, 5:28 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-807
> https://issues.apache.org/jira/browse/AURORA-807
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Reject new GC tasks when shutting down
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> d90dc68ec1526110a484cfc2b6c4658abdc2e871 
> 
> Diff: https://reviews.apache.org/r/26432/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.

[Mark Chu-Carroll]

> On Oct. 6, 2014, 12:44 p.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/client/cli/test_update.py, lines 342-353
> > 
> >
> > This sequence of mocks and writing config to a file is repeated in... 
> > many (all?) of these tests. Can you refactor to remove the repetition?

I really think that that should not be done.

For tests, I really like to be able to see, in an instant, exactly what mocks 
are being used by a particular test.  I don't want to have to look somewhere 
else; I don't want to have to mentally combine the stuff that's mocked in a 
common frame and the different stuff that's mocked and/or reconfigured in a 
particular test. The test should be as clear as it can be, and anything from 
the environment that it's mucking with should be done explicitly in the most 
local-possible context.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26363/#review55512
---


On Oct. 6, 2014, 10:57 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26363/
> ---
> 
> (Updated Oct. 6, 2014, 10:57 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-792
> https://issues.apache.org/jira/browse/aurora-792
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Make the large-update check in the client update command consider instance 
> parameters.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/context.py 
> 301531fcb443297facb78d87a18073c8b7fd4064 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 103fb53cb5101d3e025d8712e3887bccdfbb6aeb 
>   src/test/python/apache/aurora/client/cli/BUILD 
> 8ce6bd5b7faa1579372fb6935180ea982af64af8 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 85b1db19d89967a741bfba7964eeb368426f0b61 
> 
> Diff: https://reviews.apache.org/r/26363/diff/
> 
> 
> Testing
> ---
> 
> New unit tests added, all test pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[Kevin Sweeney]

> On Oct. 8, 2014, 9:19 a.m., Bill Farner wrote:
> > While our minds are on deadlock risks, it's a good idea to assess other 
> > potential vulnerabilities.
> > 
> > A quick filter to find other potential sources deserving a glance:
> > $ grep -Rl synchronized src/main/java | xargs grep -l Storage
> > src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
> > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
> > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
> > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
> > src/main/java/org/apache/aurora/scheduler/TaskVars.java
> > 
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java

My proposal is to add runtime deadlock detection for these cases via 
CycleDetectingLockFactory. I have runtime evidence that this deadlock exists 
and would like to keep this change small in scope. Happy to add this as a 
followup item to AURORA-800.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/#review55803
---


On Oct. 8, 2014, 10:27 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26422/
> ---
> 
> (Updated Oct. 8, 2014, 10:27 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-801
> https://issues.apache.org/jira/browse/AURORA-801
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Drop syncrhonized from JobUpdateEventSubscriber
> 
> This fixes a startup deadlock.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java
>  49d8b7a6c4adc4c58049c439bd09019c9e6885b1 
> 
> Diff: https://reviews.apache.org/r/26422/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Manually verified that all delegated calls to the JobUpdateController are 
> already protected by the storage write-lock.
> 
> Rather than add a potentially-flaky regression test (like the one added in 
> https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
> deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Review Request 26448: Move the error-log seed file to a user specified directory.

[Mark Chu-Carroll]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26448/
---

Review request for Aurora, David McLaughlin and Zameer Manji.


Bugs: aurora-779
https://issues.apache.org/jira/browse/aurora-779


Repository: aurora


Description
---

Instead of dumping error traces into the users current directory,
dump them into a user specified directory, with a benign default.


Diffs
-

  src/main/python/apache/aurora/client/cli/__init__.py 
5c0688f5ea184e2c0b1f8e1b54a109d8260a12fa 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
4fdcf6df493f63aae672e0834214dad208cf4110 
  src/test/python/apache/aurora/client/cli/test_create.py 
8dc0ccb78539f8b0c7829ec2927da93d4afa9ada 
  src/test/python/apache/aurora/client/cli/test_inspect.py 
fd35fe0185c1850910bd18d6a1e5831cd3effa6f 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
  src/test/python/apache/aurora/client/cli/util.py 
ff7eda20dbba073c8b24fbe3f4389292aab2d128 

Diff: https://reviews.apache.org/r/26448/diff/


Testing
---

Added new unit test; verified all tests pass.


Thanks,

Mark Chu-Carroll

Re: Review Request 26376: Implementing non-prod MTTA/R SLA metrics.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26376/#review55829
---



src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java


Seems like we're moving towards identical calculations for prod and 
non-prod tasks.  Should we just do that now and avoid the independent groupings?


- Bill Farner


On Oct. 6, 2014, 7:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26376/
> ---
> 
> (Updated Oct. 6, 2014, 7:10 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-774
> https://issues.apache.org/jira/browse/AURORA-774
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implementing non-prod MTTA/R SLA metrics.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java 
> dca680415426be2bc760875d5774c9e9399ea94b 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
> 33b6bbe4b39d516595276128bbfa0686d0bb9cbd 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> aeb90bbb822254cbe4691e45092b9581596ad800 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
> 96a04389e6b14c4d1c0bdb4d06abc046e7ea2151 
> 
> Diff: https://reviews.apache.org/r/26376/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26432: Reject new GC tasks when shutting down

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26432/#review55828
---

Ship it!


Ship It!

- David McLaughlin


On Oct. 8, 2014, 5:28 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26432/
> ---
> 
> (Updated Oct. 8, 2014, 5:28 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-807
> https://issues.apache.org/jira/browse/AURORA-807
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Reject new GC tasks when shutting down
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> d90dc68ec1526110a484cfc2b6c4658abdc2e871 
> 
> Diff: https://reviews.apache.org/r/26432/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/#review55827
---

Ship it!


Ship It!

- David McLaughlin


On Oct. 8, 2014, 5:27 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26422/
> ---
> 
> (Updated Oct. 8, 2014, 5:27 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-801
> https://issues.apache.org/jira/browse/AURORA-801
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Drop syncrhonized from JobUpdateEventSubscriber
> 
> This fixes a startup deadlock.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java
>  49d8b7a6c4adc4c58049c439bd09019c9e6885b1 
> 
> Diff: https://reviews.apache.org/r/26422/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Manually verified that all delegated calls to the JobUpdateController are 
> already protected by the storage write-lock.
> 
> Rather than add a potentially-flaky regression test (like the one added in 
> https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
> deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26432: Reject new GC tasks when shutting down

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26432/
---

(Updated Oct. 8, 2014, 10:28 a.m.)


Review request for Aurora, David McLaughlin, Bill Farner, and Brian Wickman.


Bugs: AURORA-807
https://issues.apache.org/jira/browse/AURORA-807


Repository: aurora


Description
---

Reject new GC tasks when shutting down


Diffs
-

  src/main/python/apache/aurora/executor/gc_executor.py 
80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
d90dc68ec1526110a484cfc2b6c4658abdc2e871 

Diff: https://reviews.apache.org/r/26432/diff/


Testing
---

./pants src/test/python/apache/aurora/executor:gc_executor


Thanks,

Kevin Sweeney

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[Kevin Sweeney]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/
---

(Updated Oct. 8, 2014, 10:27 a.m.)


Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.


Bugs: AURORA-801
https://issues.apache.org/jira/browse/AURORA-801


Repository: aurora


Description
---

Drop syncrhonized from JobUpdateEventSubscriber

This fixes a startup deadlock.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 
49d8b7a6c4adc4c58049c439bd09019c9e6885b1 

Diff: https://reviews.apache.org/r/26422/diff/


Testing
---

./gradlew -Pq build

Manually verified that all delegated calls to the JobUpdateController are 
already protected by the storage write-lock.

Rather than add a potentially-flaky regression test (like the one added in 
https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).


Thanks,

Kevin Sweeney

Re: Review Request 26320: Skip checkstyle on python file in 3rdparty.

[Joshua Cohen]

> On Oct. 6, 2014, 4:33 p.m., Joshua Cohen wrote:
> > *ping* Kevin.

*ping* again. Once this ships the static assets changes from 
https://github.com/jcohen/incubator-aurora/commits/jcohen/static-assets 
(reviewed at https://reviews.apache.org/r/25835/) should be mergeable.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26320/#review55511
---


On Oct. 3, 2014, 4:51 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26320/
> ---
> 
> (Updated Oct. 3, 2014, 4:51 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-780
> https://issues.apache.org/jira/browse/AURORA-780
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Skip checkstyle on python file in 3rdparty.
> 
> 
> Diffs
> -
> 
>   3rdparty/javascript/bower_components/bootstrap/test-infra/s3_cache.py 
> 472963a1e4a6c9ace6044273c7f728812aa8458b 
> 
> Diff: https://reviews.apache.org/r/26320/diff/
> 
> 
> Testing
> ---
> 
> Committed file w/ precommit hook in place.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26425: Fixing quota checking for updates.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26425/#review55819
---



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java


These methods read strangely to me: "check the quota of this task config", 
and "check the quota of this update".

How about `checkInstanceAddition`, and `checkJobUpdate`?



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java


checkArgument(instances >= 0)

(technically we could assert > 0, but this function should not care if 
instances == 0)



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java


The method names in QuotaInfo make this pretty confusing.  It's not obvious 
why `getQuota()` and `getProdConsumpgion()` are used the way they are.  To me 
this suggests the method names should be re-evaluated to avoid misuse.



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java


I would find this easier if functions were composed rather than combined.  
e.g.:

// Fetch the active production tasks in a role, group by job, compute 
resources.
Map getProdTaskResources(String role);

// Call getProdTaskResources, combine values.
IResourceAggregate getProdConsumption(String role);

// Call getProdTaskResources, calculate charge for updated job, combine 
values.
IResourceAggregate getProdConsumptionDuringUpdate(IJobUpdate update);



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java


Avoid encoding type information in the method name.  Also, observe Law of 
Demeter.  How about:

private static IResourceAggregate 
toProdResources(IJobUpdateInstructions instructions)



src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java


This is well-suited for a javadoc.


- Bill Farner


On Oct. 7, 2014, 10:03 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26425/
> ---
> 
> (Updated Oct. 7, 2014, 10:03 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-802
> https://issues.apache.org/jira/browse/AURORA-802
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Rolling up prod consumption does not work for quota checking during job 
> update intitiation where per-instance filtering is required. Splitting the 
> QuotaManager interface into 2 use cases:
> - createJob/addInstances
> - startJobUpdate
> 
> Reverting the IResourceAggregate abstraction in QuotaManager in favor of 
> task/update objects to simplify handling.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 2442b06f318c8d0cefe60296950baa1b916b42f7 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java 
> d197dd515eb646cb4babadf22e9cf6480a919060 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5dcae4a6132026504cf02093ad4c68ab521e4ab8 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b58c8f363e6e7c72accaf590b2a7cb7bf24275ea 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  02cd8f712fff3d283abf8e3eb1b4dcab1e762ac2 
> 
> Diff: https://reviews.apache.org/r/26425/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> Also, tested various update scenarios in vagrant.
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26300: Don't kill GC Executor after period of inactivity

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26300/#review55821
---

Ship it!


Ship It!

- David McLaughlin


On Oct. 3, 2014, 1:59 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26300/
> ---
> 
> (Updated Oct. 3, 2014, 1:59 a.m.)
> 
> 
> Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman.
> 
> 
> Bugs: AURORA-788
> https://issues.apache.org/jira/browse/AURORA-788
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The GC executor is configured to exit after 15 minutes of inactivity. This 
> leads to a race where the mesos slave gets a launchTask message for a GC 
> executor just as the executor has exited, causing TASK_LOST noise. This also 
> increases the risk that a slave will lose its GC executor due to the 
> scheduler not being able to find a slot for it (since GC executors will have 
> a higher churn rate).
> 
> Cluster operators will still be able to deploy new versions of the GC 
> executor as the 24-hour max lifetime limit is still in place. This patch only 
> removes the inactivity limit.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 44eb0da984a126536f0d277da3da128089201a47 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> 774c9ba0d5c31fc4c46dbe257579e013460fa943 
> 
> Diff: https://reviews.apache.org/r/26300/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26383: Health Check Disabler

[Joshua Cohen]

I'm +1 on removing the time control as well. If you need to extend the
snooze you could always touch -m the snooze file?

On Wed, Oct 8, 2014 at 9:51 AM, Bill Farner  wrote:

>
>
> > On Oct. 6, 2014, 10:40 p.m., Brian Wickman wrote:
> > > docs/configuration-reference.md, lines 359-360
> > > <
> https://reviews.apache.org/r/26383/diff/1/?file=714257#file714257line359>
> > >
> > > Is there any reason this needs to be configurable?  Why not just
> hardcode the filename as '.healthchecksnooze' and then allow the user to
> specify the snooze at runtime, e.g. echo '600' > .healthchecksnooze to
> sleep for 600 seconds.  (And if the value is malformed, just unlink and
> don't snooze.)
> >
> > David Pan wrote:
> > A few questions:
> > 1. In the case if the value is malformed, do we want to force fail
> the health check in order to alert the user the fact that they failed at
> setting the snooze duration.  Otherwise, the user might not notice the
> mistake.
> > 2. Should we read the value from the file only the first time the
> snooze file is created.  Or, should we allow the user change the value on
> the fly.
> >
> > Brian Wickman wrote:
> > 1. I think force failing the health check would be counterproductive
> -- if you accidentally fat finger, you don't want to kill the task which
> might already be a unicorn.  Instead just unlinking right away or perhaps
> using a default snooze value.
> >
> > 2. Maybe do mtime + time in the file as the snooze expiration, and
> re-read if the mtime changed.  This means at least doing a stat() each
> health check interval, but will allow you to change the snooze on the fly.
>
> How about removing the time control altogether, and let presence of the
> file serve as the snooze?  It's easier to add the time control later than
> to remove it if we decide it's unneeded.  This allows us to sidestep the
> malformed file content discussion.
>
>
> - Bill
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/#review55592
> ---
>
>
> On Oct. 6, 2014, 9:24 p.m., David Pan wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/26383/
> > ---
> >
> > (Updated Oct. 6, 2014, 9:24 p.m.)
> >
> >
> > Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> >
> >
> > Repository: aurora
> >
> >
> > Description
> > ---
> >
> > The health check disabler allows health checks for a job to be snoozed
> temporarily by touching a snooze file in the job's sandbox.  The path of
> the snooze file and the snooze duration can be set in the
> HealthCheckConfig.  The appropriate unit tests were modified/added.
> >
> > The corresponding JIRA ticket is the following:
> > https://issues.apache.org/jira/browse/AURORA-795
> >
> >
> > Diffs
> > -
> >
> >   docs/configuration-reference.md
> 5166d45ddf95ae5d8afe39dd3b00654ac91857ec
> >   docs/configuration-tutorial.md
> 67998e9dab6ac429d96d7c0d2df959336b767f32
> >   src/main/python/apache/aurora/config/schema/base.py
> f12634f103c3eb20e43f37c25d9b0fc3e3d228ec
> >   src/main/python/apache/aurora/executor/common/health_checker.py
> 4980411c847d12655cbb363404707ebd9f0bd163
> >   src/test/python/apache/aurora/executor/common/BUILD
> c7f7a003c865d479ba6e3cd7b5349322f884f653
> >   src/test/python/apache/aurora/executor/common/test_health_checker.py
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec
> >
> > Diff: https://reviews.apache.org/r/26383/diff/
> >
> >
> > Testing
> > ---
> >
> > On vagrant in ~/aurora, I ran
> > ./pants src/test/python/apache/aurora/executor::
> >
> >
> > Thanks,
> >
> > David Pan
> >
> >
>
>

Re: Review Request 26424: Disable requests http connection logging.

[Joshua Cohen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/
---

(Updated Oct. 8, 2014, 4:54 p.m.)


Review request for Aurora, Joe Smith and Bill Farner.


Changes
---

Inline.


Bugs: AURORA-770
https://issues.apache.org/jira/browse/AURORA-770


Repository: aurora


Description (updated)
---

.


Diffs (updated)
-

  src/main/python/apache/aurora/common/transport.py 
6f7c355d725b5e537cc4ae471170eaa8431da326 
  src/test/python/apache/aurora/common/test_transport.py 
c722eae2d04dec90e9c772f49c578184a2bdf76c 

Diff: https://reviews.apache.org/r/26424/diff/


Testing
---

Added hokey test, ran hokey test.

Also verified the lack of http connection logs when running client commands 
directly.


Thanks,

Joshua Cohen

Re: Review Request 26424: Disable requests http connection logging.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/#review55818
---

Ship it!


Ship It!

- Bill Farner


On Oct. 8, 2014, 4:54 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26424/
> ---
> 
> (Updated Oct. 8, 2014, 4:54 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Bill Farner.
> 
> 
> Bugs: AURORA-770
> https://issues.apache.org/jira/browse/AURORA-770
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> .
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 6f7c355d725b5e537cc4ae471170eaa8431da326 
>   src/test/python/apache/aurora/common/test_transport.py 
> c722eae2d04dec90e9c772f49c578184a2bdf76c 
> 
> Diff: https://reviews.apache.org/r/26424/diff/
> 
> 
> Testing
> ---
> 
> Added hokey test, ran hokey test.
> 
> Also verified the lack of http connection logs when running client commands 
> directly.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26424: Disable requests http connection logging.

[Joshua Cohen]

> On Oct. 8, 2014, 4:28 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/common/transport.py, line 28
> > 
> >
> > I find it odd to extract a function for this.  If i were a newcomer, i 
> > would immediately inline this function for a 3->1 line code savings.
> > 
> > If you feel strongly about extracting a function, a comment is needed 
> > to explain the thought process.
> 
> Joshua Cohen wrote:
> My thought process was, "it's already weird for initializing the 
> transport to have a side effect of globally changing the log level for 
> requests, this way if something needs to change requests logging without 
> instantiating a transport, they could just invoke this function."
> 
> Does that sound reasonable enough to keep the function? If so, I can add 
> a comment, if not I can inline.
> 
> Bill Farner wrote:
> To me, the otherwise-unused function only adds to the weirdness of it all 
> :-)
> 
> I say YAGNI - inline, add a comment that we don't like doing this, but 
> want quiet output.
> 
> Side note: is there no other wsy to do this?  It's surprising that this 
> ia a global setting in the library.

I'll inline the call. As far as I can see, there's no other way to do this. 
From the requests docs:

import requests
import logging

# these two lines enable debugging at httplib level 
(requests->urllib3->httplib)
# you will see the REQUEST, including HEADERS and DATA, and RESPONSE with 
HEADERS but without DATA.
# the only thing missing will be the response.body which is not logged.
import httplib
httplib.HTTPConnection.debuglevel = 1

logging.basicConfig() # you need to initialize logging, otherwise you will 
not see anything from requests
logging.getLogger().setLevel(logging.DEBUG)
requests_log = logging.getLogger("requests.packages.urllib3")
requests_log.setLevel(logging.DEBUG)
requests_log.propagate = True

We could do it for something other than the top-level requests logger if we 
wanted, but I think silencing requests in general is preferable.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/#review55805
---


On Oct. 7, 2014, 8:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26424/
> ---
> 
> (Updated Oct. 7, 2014, 8:32 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Bill Farner.
> 
> 
> Bugs: AURORA-770
> https://issues.apache.org/jira/browse/AURORA-770
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Disable requests http connection logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 6f7c355d725b5e537cc4ae471170eaa8431da326 
>   src/test/python/apache/aurora/common/test_transport.py 
> c722eae2d04dec90e9c772f49c578184a2bdf76c 
> 
> Diff: https://reviews.apache.org/r/26424/diff/
> 
> 
> Testing
> ---
> 
> Added hokey test, ran hokey test.
> 
> Also verified the lack of http connection logs when running client commands 
> directly.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26383: Health Check Disabler

[Bill Farner]

> On Oct. 6, 2014, 10:40 p.m., Brian Wickman wrote:
> > docs/configuration-reference.md, lines 359-360
> > 
> >
> > Is there any reason this needs to be configurable?  Why not just 
> > hardcode the filename as '.healthchecksnooze' and then allow the user to 
> > specify the snooze at runtime, e.g. echo '600' > .healthchecksnooze to 
> > sleep for 600 seconds.  (And if the value is malformed, just unlink and 
> > don't snooze.)
> 
> David Pan wrote:
> A few questions:
> 1. In the case if the value is malformed, do we want to force fail the 
> health check in order to alert the user the fact that they failed at setting 
> the snooze duration.  Otherwise, the user might not notice the mistake.
> 2. Should we read the value from the file only the first time the snooze 
> file is created.  Or, should we allow the user change the value on the fly.
> 
> Brian Wickman wrote:
> 1. I think force failing the health check would be counterproductive -- 
> if you accidentally fat finger, you don't want to kill the task which might 
> already be a unicorn.  Instead just unlinking right away or perhaps using a 
> default snooze value.
> 
> 2. Maybe do mtime + time in the file as the snooze expiration, and 
> re-read if the mtime changed.  This means at least doing a stat() each health 
> check interval, but will allow you to change the snooze on the fly.

How about removing the time control altogether, and let presence of the file 
serve as the snooze?  It's easier to add the time control later than to remove 
it if we decide it's unneeded.  This allows us to sidestep the malformed file 
content discussion.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/#review55592
---


On Oct. 6, 2014, 9:24 p.m., David Pan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/
> ---
> 
> (Updated Oct. 6, 2014, 9:24 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The health check disabler allows health checks for a job to be snoozed 
> temporarily by touching a snooze file in the job's sandbox.  The path of the 
> snooze file and the snooze duration can be set in the HealthCheckConfig.  The 
> appropriate unit tests were modified/added.
> 
> The corresponding JIRA ticket is the following:
> https://issues.apache.org/jira/browse/AURORA-795
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
>   docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 
>   src/main/python/apache/aurora/config/schema/base.py 
> f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 4980411c847d12655cbb363404707ebd9f0bd163 
>   src/test/python/apache/aurora/executor/common/BUILD 
> c7f7a003c865d479ba6e3cd7b5349322f884f653 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec 
> 
> Diff: https://reviews.apache.org/r/26383/diff/
> 
> 
> Testing
> ---
> 
> On vagrant in ~/aurora, I ran
> ./pants src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> David Pan
> 
>

Re: Review Request 26328: Improve handling of unknown errors in the aurora client.

[Maxim Khutornenko]

> On Oct. 8, 2014, 1:05 a.m., Maxim Khutornenko wrote:
> >

I see this got committed even though there are still open questions in this RB. 
Any chance these can get answered?


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26328/#review55752
---


On Oct. 8, 2014, 2:42 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26328/
> ---
> 
> (Updated Oct. 8, 2014, 2:42 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-779
> https://issues.apache.org/jira/browse/aurora-779
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve handling of unknown errors in the aurora client.
> 
> Instead of dumping the stack on the user's terminal, or
> absorbing the error and generating a brief error
> message, the client now writes detailed information about
> the error is written into an error log file, and the
> user is given a clean error message referring them to that
> file for details.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> e0c3050151bca1128ed7e476ec5133407a20f6c2 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 7c0975ce9d415b19b6704b9a772ee2619ac9a2af 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
> 
> Diff: https://reviews.apache.org/r/26328/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26383: Health Check Disabler

[Bill Farner]

> On Oct. 6, 2014, 10:55 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 69
> > 
> >
> > Do we need to make this path configurable? I'm trying to think of a 
> > reason that someone might want to change it, and I'm coming up blank, 
> > however I could envision a scenario where someone does something malicious 
> > (intentionally or not) like sets this path to something that shouldn't be 
> > removed (e.g. the artifact being run).

+1, knob uneeded


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26383/#review55590
---


On Oct. 6, 2014, 9:24 p.m., David Pan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26383/
> ---
> 
> (Updated Oct. 6, 2014, 9:24 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Brian Wickman, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The health check disabler allows health checks for a job to be snoozed 
> temporarily by touching a snooze file in the job's sandbox.  The path of the 
> snooze file and the snooze duration can be set in the HealthCheckConfig.  The 
> appropriate unit tests were modified/added.
> 
> The corresponding JIRA ticket is the following:
> https://issues.apache.org/jira/browse/AURORA-795
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 5166d45ddf95ae5d8afe39dd3b00654ac91857ec 
>   docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 
>   src/main/python/apache/aurora/config/schema/base.py 
> f12634f103c3eb20e43f37c25d9b0fc3e3d228ec 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 4980411c847d12655cbb363404707ebd9f0bd163 
>   src/test/python/apache/aurora/executor/common/BUILD 
> c7f7a003c865d479ba6e3cd7b5349322f884f653 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> aa36415fa891fc523a3a376ffeca5d3cd5ceabec 
> 
> Diff: https://reviews.apache.org/r/26383/diff/
> 
> 
> Testing
> ---
> 
> On vagrant in ~/aurora, I ran
> ./pants src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> David Pan
> 
>

Re: Review Request 26424: Disable requests http connection logging.

[Bill Farner]

> On Oct. 8, 2014, 4:28 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/common/transport.py, line 28
> > 
> >
> > I find it odd to extract a function for this.  If i were a newcomer, i 
> > would immediately inline this function for a 3->1 line code savings.
> > 
> > If you feel strongly about extracting a function, a comment is needed 
> > to explain the thought process.
> 
> Joshua Cohen wrote:
> My thought process was, "it's already weird for initializing the 
> transport to have a side effect of globally changing the log level for 
> requests, this way if something needs to change requests logging without 
> instantiating a transport, they could just invoke this function."
> 
> Does that sound reasonable enough to keep the function? If so, I can add 
> a comment, if not I can inline.

To me, the otherwise-unused function only adds to the weirdness of it all :-)

I say YAGNI - inline, add a comment that we don't like doing this, but want 
quiet output.

Side note: is there no other wsy to do this?  It's surprising that this ia a 
global setting in the library.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/#review55805
---


On Oct. 7, 2014, 8:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26424/
> ---
> 
> (Updated Oct. 7, 2014, 8:32 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Bill Farner.
> 
> 
> Bugs: AURORA-770
> https://issues.apache.org/jira/browse/AURORA-770
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Disable requests http connection logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 6f7c355d725b5e537cc4ae471170eaa8431da326 
>   src/test/python/apache/aurora/common/test_transport.py 
> c722eae2d04dec90e9c772f49c578184a2bdf76c 
> 
> Diff: https://reviews.apache.org/r/26424/diff/
> 
> 
> Testing
> ---
> 
> Added hokey test, ran hokey test.
> 
> Also verified the lack of http connection logs when running client commands 
> directly.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26432: Reject new GC tasks when shutting down

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26432/#review55810
---

Ship it!


Ship It!

- Bill Farner


On Oct. 8, 2014, 12:14 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26432/
> ---
> 
> (Updated Oct. 8, 2014, 12:14 a.m.)
> 
> 
> Review request for Aurora and Brian Wickman.
> 
> 
> Bugs: AURORA-807
> https://issues.apache.org/jira/browse/AURORA-807
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Reject new GC tasks when shutting down
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 80ce6dc93e17f01c95a84ee8bef2ce87cafa88c0 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> d90dc68ec1526110a484cfc2b6c4658abdc2e871 
> 
> Diff: https://reviews.apache.org/r/26432/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26424: Disable requests http connection logging.

[Joshua Cohen]

> On Oct. 8, 2014, 4:28 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/common/transport.py, line 28
> > 
> >
> > I find it odd to extract a function for this.  If i were a newcomer, i 
> > would immediately inline this function for a 3->1 line code savings.
> > 
> > If you feel strongly about extracting a function, a comment is needed 
> > to explain the thought process.

My thought process was, "it's already weird for initializing the transport to 
have a side effect of globally changing the log level for requests, this way if 
something needs to change requests logging without instantiating a transport, 
they could just invoke this function."

Does that sound reasonable enough to keep the function? If so, I can add a 
comment, if not I can inline.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/#review55805
---


On Oct. 7, 2014, 8:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26424/
> ---
> 
> (Updated Oct. 7, 2014, 8:32 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Bill Farner.
> 
> 
> Bugs: AURORA-770
> https://issues.apache.org/jira/browse/AURORA-770
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Disable requests http connection logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 6f7c355d725b5e537cc4ae471170eaa8431da326 
>   src/test/python/apache/aurora/common/test_transport.py 
> c722eae2d04dec90e9c772f49c578184a2bdf76c 
> 
> Diff: https://reviews.apache.org/r/26424/diff/
> 
> 
> Testing
> ---
> 
> Added hokey test, ran hokey test.
> 
> Also verified the lack of http connection logs when running client commands 
> directly.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26424: Disable requests http connection logging.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26424/#review55805
---



src/main/python/apache/aurora/common/transport.py


I find it odd to extract a function for this.  If i were a newcomer, i 
would immediately inline this function for a 3->1 line code savings.

If you feel strongly about extracting a function, a comment is needed to 
explain the thought process.


- Bill Farner


On Oct. 7, 2014, 8:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26424/
> ---
> 
> (Updated Oct. 7, 2014, 8:32 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Bill Farner.
> 
> 
> Bugs: AURORA-770
> https://issues.apache.org/jira/browse/AURORA-770
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Disable requests http connection logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 6f7c355d725b5e537cc4ae471170eaa8431da326 
>   src/test/python/apache/aurora/common/test_transport.py 
> c722eae2d04dec90e9c772f49c578184a2bdf76c 
> 
> Diff: https://reviews.apache.org/r/26424/diff/
> 
> 
> Testing
> ---
> 
> Added hokey test, ran hokey test.
> 
> Also verified the lack of http connection logs when running client commands 
> directly.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 26422: Drop syncrhonized from JobUpdateEventSubscriber

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26422/#review55803
---


While our minds are on deadlock risks, it's a good idea to assess other 
potential vulnerabilities.

A quick filter to find other potential sources deserving a glance:
$ grep -Rl synchronized src/main/java | xargs grep -l Storage
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
src/main/java/org/apache/aurora/scheduler/TaskVars.java

src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java

- Bill Farner


On Oct. 7, 2014, 7:28 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26422/
> ---
> 
> (Updated Oct. 7, 2014, 7:28 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-801
> https://issues.apache.org/jira/browse/AURORA-801
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Drop syncrhonized from JobUpdateEventSubscriber
> 
> This fixes a startup deadlock.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java
>  49d8b7a6c4adc4c58049c439bd09019c9e6885b1 
> 
> Diff: https://reviews.apache.org/r/26422/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> Manually verified that all delegated calls to the JobUpdateController are 
> already protected by the storage write-lock.
> 
> Rather than add a potentially-flaky regression test (like the one added in 
> https://reviews.apache.org/r/25556/) I'd prefer to prioritize adding runtime 
> deadlock detection (https://issues.apache.org/jira/browse/AURORA-800).
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Review Request 26445: Fix error in incorrect deprecation warning on v1 "ssh" command.

[Mark Chu-Carroll]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26445/
---

Review request for Aurora, David McLaughlin and Zameer Manji.


Bugs: aurora-804
https://issues.apache.org/jira/browse/aurora-804


Repository: aurora


Description
---

The error message incorrectly displayed a list-value for the
"--tunnels" parameter, when in fact it should expand the list
into multiple instances of "--tunnels".


Diffs
-

  src/main/python/apache/aurora/client/commands/ssh.py 
9ee94a0bdfd8bbc030ae978e2ac3fe39571b4ce2 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 

Diff: https://reviews.apache.org/r/26445/diff/


Testing
---

New unit test.


Thanks,

Mark Chu-Carroll

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

> On Oct. 7, 2014, 9:18 p.m., Kevin Sweeney wrote:
> > src/test/python/apache/aurora/client/cli/test_api_from_cli.py, line 51
> > 
> >
> > Why do these need to be mocks? This would work identically using the 
> > Thrift structures directly.
> > 
> > If you want to prevent setting a field that doesn't exist you can use 
> > the constructor kwargs form of the thrift object, i.e.
> > 
> > ```py
> > task = ScheduledTask(
> >   key=JobKey(role=..., ...),
> >   assignedTask=AssignedTask(
> > slaveHost=...,
> > ...
> >   ),
> >   ...
> > )
> > ```

I thought the same way when i came to this code, but had already pulled a long 
thread with this diff and didn't want to continue further for the sake of 
reviewer sanity.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26308/#review55712
---


On Oct. 4, 2014, 5:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26308/
> ---
> 
> (Updated Oct. 4, 2014, 5:55 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes two problems:
> 
> - mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
> objects were being passed around and somehow resulted in the test case 
> passing.
> - use of `threading.Event()` was broken in scheduler_client.py.  I don't 
> think it's possible to enter those branches.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 26300792594e4005dacc139a9f89711b8a66ab61 
>   src/main/python/apache/aurora/client/api/command_runner.py 
> a1fed5fc75dde3a79c840515e6daa4741156ef97 
>   src/main/python/apache/aurora/client/api/job_monitor.py 
> 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 311c954f1db245b75192d00c6aca0721085fbf32 
>   src/main/python/apache/aurora/client/api/updater.py 
> bf608981c2f2e7960b68c3fbda144277a59a3d40 
>   src/main/python/apache/aurora/common/aurora_job_key.py 
> a7ca7b6df6b9566b3ce617283ccac948deb2eb83 
>   src/test/python/apache/aurora/client/api/test_job_monitor.py 
> 5b26539f86a0a82f72753a803a769eda77cbc332 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1835843f1795b0530874ec561582df17acfbce65 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 6905831b23a84320e7f41843efd62b86da366c0b 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_cancel_update.py 
> c15e142930c9474c7873dd931261b6ab4eb5967f 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> e1a6f764830e06c73d0bc10a3b5da67219da836c 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> e3a366bf67074e50787394cad58d5e01359b641e 
>   src/test/python/apache/aurora/client/cli/test_logging.py 
> 6285fbb07442291c2dc4096e68eb285c98994097 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 1ce9a632874e818eee71573cd481842affae3615 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 85b1db19d89967a741bfba7964eeb368426f0b61 
>   src/test/python/apache/aurora/client/commands/test_admin.py 
> 1192556c027dc3adf16bb37adeac7798cf9ef93d 
>   src/test/python/apache/aurora/client/commands/test_cancel_update.py 
> 5f05ef7c0643d189de3de38c75aae58c2a3814a4 
>   src/test/python/apache/aurora/client/commands/test_create.py 
> 7503345ea1c0f224a894ce02cc2c2d8719574e32 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> 8f5da7d2bca9b0486b635afe49d3885151624e12 
>   src/test/python/apache/aurora/client/commands/test_hooks.py 
> 0861f13b13a8406950ba953efba0ffae186a8253 
>   src/test/python/apache/aurora/client/commands/test_kill.py 
> c0a6fd44c5691cde50746ffdec325bb11a33469a 
>   src/test/python/apache/aurora/client/commands/test_run.py 
> e97b5156609f80a4170028e780bcd891e56983ff 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> bda1f28d544ae48428129f8167d8632ef27f5fba

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

> On Oct. 7, 2014, 9:39 p.m., Mark Chu-Carroll wrote:
> > Looks good.
> > 
> > One note on the change description: I'm willing to bet that there is a way 
> > to get that branch active in a test. Every single time that I've ever said 
> > that something couldn't be tested, or that some branch couldn't be 
> > exercised in a test, I've always been wrong. There's a way.

Poor wording on my part.  I meant to say that it was not possible to enter 
those branches before this diff, meaning we would continue to loop when an 
exception was caught.  With this diff, those branches are now entered in 
pracice and in the test cases changed.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26308/#review55716
---


On Oct. 4, 2014, 5:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26308/
> ---
> 
> (Updated Oct. 4, 2014, 5:55 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes two problems:
> 
> - mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
> objects were being passed around and somehow resulted in the test case 
> passing.
> - use of `threading.Event()` was broken in scheduler_client.py.  I don't 
> think it's possible to enter those branches.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 26300792594e4005dacc139a9f89711b8a66ab61 
>   src/main/python/apache/aurora/client/api/command_runner.py 
> a1fed5fc75dde3a79c840515e6daa4741156ef97 
>   src/main/python/apache/aurora/client/api/job_monitor.py 
> 18d5c0381d43fc7b24bae4b2e5e6fdc774a74b52 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 311c954f1db245b75192d00c6aca0721085fbf32 
>   src/main/python/apache/aurora/client/api/updater.py 
> bf608981c2f2e7960b68c3fbda144277a59a3d40 
>   src/main/python/apache/aurora/common/aurora_job_key.py 
> a7ca7b6df6b9566b3ce617283ccac948deb2eb83 
>   src/test/python/apache/aurora/client/api/test_job_monitor.py 
> 5b26539f86a0a82f72753a803a769eda77cbc332 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1835843f1795b0530874ec561582df17acfbce65 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 6905831b23a84320e7f41843efd62b86da366c0b 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_cancel_update.py 
> c15e142930c9474c7873dd931261b6ab4eb5967f 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> e1a6f764830e06c73d0bc10a3b5da67219da836c 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> e3a366bf67074e50787394cad58d5e01359b641e 
>   src/test/python/apache/aurora/client/cli/test_logging.py 
> 6285fbb07442291c2dc4096e68eb285c98994097 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> bd30b11022ee0d1fb38da9f6efd4d9c5923b7d13 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 1ce9a632874e818eee71573cd481842affae3615 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 85b1db19d89967a741bfba7964eeb368426f0b61 
>   src/test/python/apache/aurora/client/commands/test_admin.py 
> 1192556c027dc3adf16bb37adeac7798cf9ef93d 
>   src/test/python/apache/aurora/client/commands/test_cancel_update.py 
> 5f05ef7c0643d189de3de38c75aae58c2a3814a4 
>   src/test/python/apache/aurora/client/commands/test_create.py 
> 7503345ea1c0f224a894ce02cc2c2d8719574e32 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> 8f5da7d2bca9b0486b635afe49d3885151624e12 
>   src/test/python/apache/aurora/client/commands/test_hooks.py 
> 0861f13b13a8406950ba953efba0ffae186a8253 
>   src/test/python/apache/aurora/client/commands/test_kill.py 
> c0a6fd44c5691cde50746ffdec325bb11a33469a 
>   src/test/python/apache/aurora/client/commands/test_run.py 
> e97b5156609f80a4170028e780bcd891e56983ff 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> c5ca66e378bfc97c40a406a758ae4dfaef8ab2c8 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> bda1f28d544ae48428129f8167d8632ef27f5fba 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> af2cbc7f88287201a472ba36902b00d90bc77d3b 
> 
> Diff: https://reviews.apache.org/r/26308/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:a

Re: Review Request 26328: Improve handling of unknown errors in the aurora client.

[David McLaughlin]

> On Oct. 8, 2014, 1:58 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/cli/__init__.py, line 435
> > 
> >
> > It writes into the current directory where the user is executing the 
> > client.

This is going to lead to complaints. On the dev list, John wrote:

"The full backtrace goes off to a file in the user's home dir somewhere and
then you can ask them to run a command passing the pill ref to get the full
error report without worry of re-running some non-idempotent command, etc."

So I really think we should write them to a consistent and predictable location.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26328/#review55794
---


On Oct. 8, 2014, 2:42 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26328/
> ---
> 
> (Updated Oct. 8, 2014, 2:42 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-779
> https://issues.apache.org/jira/browse/aurora-779
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve handling of unknown errors in the aurora client.
> 
> Instead of dumping the stack on the user's terminal, or
> absorbing the error and generating a brief error
> message, the client now writes detailed information about
> the error is written into an error log file, and the
> user is given a clean error message referring them to that
> file for details.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> e0c3050151bca1128ed7e476ec5133407a20f6c2 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 7c0975ce9d415b19b6704b9a772ee2619ac9a2af 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
> 
> Diff: https://reviews.apache.org/r/26328/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26328: Improve handling of unknown errors in the aurora client.

[Mark Chu-Carroll]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26328/
---

(Updated Oct. 8, 2014, 10:42 a.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

Rebase.


Bugs: aurora-779
https://issues.apache.org/jira/browse/aurora-779


Repository: aurora


Description
---

Improve handling of unknown errors in the aurora client.

Instead of dumping the stack on the user's terminal, or
absorbing the error and generating a brief error
message, the client now writes detailed information about
the error is written into an error log file, and the
user is given a clean error message referring them to that
file for details.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/__init__.py 
e0c3050151bca1128ed7e476ec5133407a20f6c2 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
7c0975ce9d415b19b6704b9a772ee2619ac9a2af 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
78f21d2f20cf71fa2dfe0614885d44d2948decd2 
  src/test/python/apache/aurora/client/cli/test_create.py 
6e55188bdfc576506848605debb391288e696fe3 

Diff: https://reviews.apache.org/r/26328/diff/


Testing
---

New unit test.


Thanks,

Mark Chu-Carroll

Re: Review Request 26328: Improve handling of unknown errors in the aurora client.

[Mark Chu-Carroll]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26328/#review55794
---



src/main/python/apache/aurora/client/cli/__init__.py


It writes into the current directory where the user is executing the client.


- Mark Chu-Carroll


On Oct. 6, 2014, 2:55 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26328/
> ---
> 
> (Updated Oct. 6, 2014, 2:55 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-779
> https://issues.apache.org/jira/browse/aurora-779
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve handling of unknown errors in the aurora client.
> 
> Instead of dumping the stack on the user's terminal, or
> absorbing the error and generating a brief error
> message, the client now writes detailed information about
> the error is written into an error log file, and the
> user is given a clean error message referring them to that
> file for details.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> e0c3050151bca1128ed7e476ec5133407a20f6c2 
>   src/main/python/apache/aurora/client/cli/context.py 
> 301531fcb443297facb78d87a18073c8b7fd4064 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 7c0975ce9d415b19b6704b9a772ee2619ac9a2af 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 6e55188bdfc576506848605debb391288e696fe3 
>   src/test/python/apache/aurora/client/cli/util.py 
> e1ee884c06f3bc22bcd9e908ff61af9459a0b535 
> 
> Diff: https://reviews.apache.org/r/26328/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>