Re: Review Request 22457: Improve aurora job diff command.

2014-10-24 Thread Maxim Khutornenko

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



src/main/python/apache/aurora/client/cli/jobs.py
https://reviews.apache.org/r/22457/#comment99224

This is no longer true. The `populated` field reprsents a single TaskConfig 
now. You'd need to inflate a task config list according to the local instance 
count.



src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/22457/#comment99225

Please, refactor the correspondent method in util.py to give you a single 
scheduled task. There is nothing unique here that cannot be reused.



src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/22457/#comment99228

A ScheduledTask does not have a `key` field. Please, avoid using dynamic 
properties as they are very confusing in test.



src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/22457/#comment99226

These are not mocks anymore, please update method name.



src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/22457/#comment99227

same here



src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/22457/#comment99229

Missing `job` field as a future replacment to owner/environment/jobName.



src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/22457/#comment99230

Is this still needed? If so, would be great to have a bit of formatting 
around it to really help debugging when needed.


- Maxim Khutornenko


On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/22457/
 ---
 
 (Updated Oct. 24, 2014, 2:50 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Brian Wickman.
 
 
 Bugs: aurora-520
 https://issues.apache.org/jira/browse/aurora-520
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add a new diff method, which uses field-by-field comparison of JSON trees for 
 comparing running job configurations to potentially updated configs.
 
 - Allow exclusion of semantically irrelevant fields.
 - Provide a clearer list of the differences between configs.
 - Provide a scripting-friendly alternative JSON syntax for diffs.
 
 The old diff behavior is still available under the --use-shell-diff option.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/BUILD 
 995570325bbb09ecbcc2ace5d223760c5d49367f 
   src/main/python/apache/aurora/client/cli/jobs.py 
 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
   src/test/python/apache/aurora/client/cli/BUILD 
 4692d31a9c128664273f71d15ee217dc060e66f0 
   src/test/python/apache/aurora/client/cli/test_diff.py 
 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
   src/test/python/apache/aurora/client/cli/test_json_diff.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/22457/diff/
 
 
 Testing
 ---
 
 New unit tests of the JSON tree diff code, plus a bunch of new job diff 
 tests of the new functionality.
 All tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 22457: Improve aurora job diff command.

2014-10-24 Thread Aurora ReviewBot

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


Master (53f4e73) is red with this patch.
  ./build-support/jenkins/build.sh

src.test.python.apache.aurora.client.api.api
.   SUCCESS
src.test.python.apache.aurora.client.api.disambiguator  
.   SUCCESS
src.test.python.apache.aurora.client.api.instance_watcher   
.   SUCCESS
src.test.python.apache.aurora.client.api.job_monitor
.   SUCCESS
src.test.python.apache.aurora.client.api.mux
.   SUCCESS
src.test.python.apache.aurora.client.api.quota_check
.   SUCCESS
src.test.python.apache.aurora.client.api.restarter  
.   SUCCESS
src.test.python.apache.aurora.client.api.scheduler_client   
.   SUCCESS
src.test.python.apache.aurora.client.api.sla
.   SUCCESS
src.test.python.apache.aurora.client.api.updater
.   SUCCESS
src.test.python.apache.aurora.client.api.updater_util   
.   SUCCESS
src.test.python.apache.aurora.client.binding_helper 
.   SUCCESS
src.test.python.apache.aurora.client.cli.api
.   SUCCESS
src.test.python.apache.aurora.client.cli.bridge 
.   SUCCESS
src.test.python.apache.aurora.client.cli.command_hooks  
.   SUCCESS
src.test.python.apache.aurora.client.cli.cron   
.   SUCCESS
src.test.python.apache.aurora.client.cli.help   
.   SUCCESS
src.test.python.apache.aurora.client.cli.inspect
.   SUCCESS
src.test.python.apache.aurora.client.cli.job
.   FAILURE
src.test.python.apache.aurora.client.config 
.   SUCCESS

- Aurora ReviewBot


On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/22457/
 ---
 
 (Updated Oct. 24, 2014, 2:50 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Brian Wickman.
 
 
 Bugs: aurora-520
 https://issues.apache.org/jira/browse/aurora-520
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add a new diff method, which uses field-by-field comparison of JSON trees for 
 comparing running job configurations to potentially updated configs.
 
 - Allow exclusion of semantically irrelevant fields.
 - Provide a clearer list of the differences between configs.
 - Provide a scripting-friendly alternative JSON syntax for diffs.
 
 The old diff behavior is still available under the --use-shell-diff option.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/BUILD 
 995570325bbb09ecbcc2ace5d223760c5d49367f 
   src/main/python/apache/aurora/client/cli/jobs.py 
 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
   src/test/python/apache/aurora/client/cli/BUILD 
 4692d31a9c128664273f71d15ee217dc060e66f0 
   src/test/python/apache/aurora/client/cli/test_diff.py 
 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
   src/test/python/apache/aurora/client/cli/test_json_diff.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/22457/diff/
 
 
 Testing
 ---
 
 New unit tests of the JSON tree diff code, plus a bunch of new job diff 
 tests of the new functionality.
 All tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 27129: Upgrade psutil to 2.1.3

2014-10-24 Thread Aurora ReviewBot

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


LGTM! Master (53f4e73) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 24, 2014, 2:13 a.m., Joe Smith wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27129/
 ---
 
 (Updated Oct. 24, 2014, 2:13 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Brian Wickman.
 
 
 Bugs: AURORA-627
 https://issues.apache.org/jira/browse/AURORA-627
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Upgrade psutil to 2.1.3
 
 
 Diffs
 -
 
   3rdparty/python/BUILD 4493bf1ec763f367d1549f84e9a627a35254bcc5 
 
 Diff: https://reviews.apache.org/r/27129/diff/
 
 
 Testing
 ---
 
 e2e tests and on OS X 10.10 and from within the vagrant image:
 
 src.test.python.apache.aurora.admin.admin_util
   .   SUCCESS
 src.test.python.apache.aurora.admin.host_maintenance  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.api  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.disambiguator
   .   SUCCESS
 src.test.python.apache.aurora.client.api.instance_watcher 
   .   SUCCESS
 src.test.python.apache.aurora.client.api.job_monitor  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.mux  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.quota_check  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.restarter
   .   SUCCESS
 src.test.python.apache.aurora.client.api.scheduler_client 
   .   SUCCESS
 src.test.python.apache.aurora.client.api.sla  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.updater  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.updater_util 
   .   SUCCESS
 src.test.python.apache.aurora.client.binding_helper   
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.api  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.bridge   
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.command_hooks
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.config   
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.cron 
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.help 
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.inspect  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.job  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.logging  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.plugins  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.quota
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.sla  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.supdate  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.task 
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.update   
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.admin   
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.core
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.hooks   
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.maintenance 
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.run 
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.ssh 
   .   SUCCESS
 src.test.python.apache.aurora.client.config   
   .   SUCCESS
 src.test.python.apache.aurora.client.hooks.hooked_api 
   .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api 
   .   SUCCESS
 src.test.python.apache.aurora.common.test_aurora_job_key  
   .   SUCCESS
 

Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Aurora ReviewBot

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


LGTM! Master (53f4e73) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 24, 2014, 1:50 a.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 1:50 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Joshua Cohen

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


Does it make sense to use the ReviewBoard Python client from rbtools? 
https://www.reviewboard.org/docs/rbtools/0.5/api/overview/

What is the plan to actually run this script? Is it safe to assume that it will 
be executed from an up to date git repo, or will it need to git pull?


build-support/jenkins/review_feedback.py
https://reviews.apache.org/r/27145/#comment99248

Our Python continuation indent style is generally the same as our Java 
style, so these should be indented 4 past the parent, not aligned?


- Joshua Cohen


On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 5:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Zameer Manji

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

(Updated Oct. 24, 2014, 12:59 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Bill's feedback.


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


Repository: aurora


Description
---

Cache the host's maintenance status with the offer. By caching the status
alongside the offer the scheduler does not need to access the attribute store
for every offer it considers for a task.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
92c843830df7a779abace38bb0ce84d4cbeb5af4 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
4088559c39014befaddb0b29dad45fac9f4545c4 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
da29428adaebcb27b20a10a8c6b7e380662fce4a 
  src/main/java/org/apache/aurora/scheduler/http/Offers.java 
b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
4db9be86f2e7db08d12e0182914a7c5130301b13 
  src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
  src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
  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/events/NotifyingSchedulingFilterTest.java
 4065629e9d488b122aa811b9802def0b51a21294 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
6a9c4ee278ed3ee8222404504e571f20991c2ae2 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
c48cbae4864127e7799917182439f7670285b0d3 

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


Testing
---

./gradlew clean build -Pq


Thanks,

Zameer Manji



Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Zameer Manji

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



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
https://reviews.apache.org/r/27100/#comment99292

I don't understand what you mean by matches what we do for dedicated host 
mismatches as well.


- Zameer Manji


On Oct. 23, 2014, 6:50 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 23, 2014, 6:50 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread David McLaughlin

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

(Updated Oct. 24, 2014, 8:09 p.m.)


Review request for Aurora, Mark Chu-Carroll and Zameer Manji.


Changes
---

rebase.


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


Repository: aurora


Description
---

Use of Mock() without a specification considered harmful. I went through and 
updated as many mocks as I could. 

Any remaining can be classified as:

1) Mocks of classes that cannot be spec'd. Almost all instances of 
SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 
2) Primitives like strings and callback functions or data objects like dicts 
and pystachio structs.
3) Weird mocks that broke code where they really shouldn't have (off the top of 
my head - in test_diff.py and commands/test_run.py) - both when they were 
spec'd and when they were replaced with real thrift structs. 


The remaining offenders:

$ grep -r --include=*.py Mock() src/test/python 
src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback 
= mock.Mock()
src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback 
= mock.Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
Mock()
src/test/python/apache/aurora/client/api/test_job_monitor.py:
self._scheduler = Mock()
src/test/python/apache/aurora/client/api/test_quota_check.py:
self._scheduler = Mock()
src/test/python/apache/aurora/client/api/test_scheduler_client.py:
client._connect_scheduler = mock.MagicMock()
src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
Mock()
src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:  
job.assignedTask.task.executorConfig.data = Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock()
src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock()
src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = 
Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = 
Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = 
Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:  job = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:  
job.assignedTask.task.executorConfig.data = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_listjobs.py:mock_options 
= Mock()
src/test/python/apache/aurora/client/commands/test_listjobs.py:  job = 
Mock()
src/test/python/apache/aurora/client/commands/test_maintenance.py:
mock_callback = Mock()
src/test/python/apache/aurora/client/commands/test_maintenance.py:  
mock_wait = Mock()
src/test/python/apache/aurora/client/commands/util.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = 
Mock()


Diffs (updated)
-

  src/test/python/apache/aurora/admin/test_admin_util.py 
f5c8c69c1109d15ee3886fb863014c3285240db1 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
60c75300501c36ac20a97f78ff18b3ca7af30699 
  src/test/python/apache/aurora/client/cli/test_cron.py 
c7b71c29d44150162fec8066947623fa91815424 
  src/test/python/apache/aurora/client/cli/test_diff.py 

Re: Review Request 27129: Upgrade psutil to 2.1.3

2014-10-24 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 23, 2014, 7:13 p.m., Joe Smith wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27129/
 ---
 
 (Updated Oct. 23, 2014, 7:13 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Brian Wickman.
 
 
 Bugs: AURORA-627
 https://issues.apache.org/jira/browse/AURORA-627
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Upgrade psutil to 2.1.3
 
 
 Diffs
 -
 
   3rdparty/python/BUILD 4493bf1ec763f367d1549f84e9a627a35254bcc5 
 
 Diff: https://reviews.apache.org/r/27129/diff/
 
 
 Testing
 ---
 
 e2e tests and on OS X 10.10 and from within the vagrant image:
 
 src.test.python.apache.aurora.admin.admin_util
   .   SUCCESS
 src.test.python.apache.aurora.admin.host_maintenance  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.api  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.disambiguator
   .   SUCCESS
 src.test.python.apache.aurora.client.api.instance_watcher 
   .   SUCCESS
 src.test.python.apache.aurora.client.api.job_monitor  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.mux  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.quota_check  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.restarter
   .   SUCCESS
 src.test.python.apache.aurora.client.api.scheduler_client 
   .   SUCCESS
 src.test.python.apache.aurora.client.api.sla  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.updater  
   .   SUCCESS
 src.test.python.apache.aurora.client.api.updater_util 
   .   SUCCESS
 src.test.python.apache.aurora.client.binding_helper   
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.api  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.bridge   
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.command_hooks
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.config   
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.cron 
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.help 
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.inspect  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.job  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.logging  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.plugins  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.quota
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.sla  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.supdate  
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.task 
   .   SUCCESS
 src.test.python.apache.aurora.client.cli.update   
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.admin   
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.core
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.hooks   
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.maintenance 
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.run 
   .   SUCCESS
 src.test.python.apache.aurora.client.commands.ssh 
   .   SUCCESS
 src.test.python.apache.aurora.client.config   
   .   SUCCESS
 src.test.python.apache.aurora.client.hooks.hooked_api 
   .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api 
   .   SUCCESS
 src.test.python.apache.aurora.common.test_aurora_job_key  
   .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster 
   .   SUCCESS
 

Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Maxim Khutornenko

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

Ship it!



build-support/jenkins/review_feedback.py
https://reviews.apache.org/r/27145/#comment99252

replace with ternary?



build-support/jenkins/review_feedback.py
https://reviews.apache.org/r/27145/#comment99253

Is there a legitimate case to allow non 200 response code here?



build-support/jenkins/review_feedback.py
https://reviews.apache.org/r/27145/#comment99254

Combine with the above print?


- Maxim Khutornenko


On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 5:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Bill Farner


 On Oct. 24, 2014, 5:57 p.m., Joshua Cohen wrote:
  Does it make sense to use the ReviewBoard Python client from rbtools? 
  https://www.reviewboard.org/docs/rbtools/0.5/api/overview/
  
  What is the plan to actually run this script? Is it safe to assume that it 
  will be executed from an up to date git repo, or will it need to git pull?

I'm hesitant, mostly because i don't think the code savings is worth the cost 
of adding build infrastructure around this script.  I could be swayed, though.


 On Oct. 24, 2014, 5:57 p.m., Joshua Cohen wrote:
  build-support/jenkins/review_feedback.py, lines 98-100
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line98
 
  Our Python continuation indent style is generally the same as our Java 
  style, so these should be indented 4 past the parent, not aligned?

Fixed.


- Bill


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


On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 5:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Maxim Khutornenko

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


It's not obvious from the diff: where do we gain perf? Can you point to the 
place where we don't do store calls anymore?

- Maxim Khutornenko


On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 7:59 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Oct. 24, 2014, 1:09 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27058/
 ---
 
 (Updated Oct. 24, 2014, 1:09 p.m.)
 
 
 Review request for Aurora, Mark Chu-Carroll and Zameer Manji.
 
 
 Bugs: AURORA-248
 https://issues.apache.org/jira/browse/AURORA-248
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use of Mock() without a specification considered harmful. I went through and 
 updated as many mocks as I could. 
 
 Any remaining can be classified as:
 
 1) Mocks of classes that cannot be spec'd. Almost all instances of 
 SchedulerProxy, which uses __getattr__ to delegate to the read or write 
 client. 
 2) Primitives like strings and callback functions or data objects like dicts 
 and pystachio structs.
 3) Weird mocks that broke code where they really shouldn't have (off the top 
 of my head - in test_diff.py and commands/test_run.py) - both when they were 
 spec'd and when they were replaced with real thrift structs. 
 
 
 The remaining offenders:
 
 $ grep -r --include=*.py Mock() src/test/python 
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
 Mock()
 src/test/python/apache/aurora/client/api/test_job_monitor.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_quota_check.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_scheduler_client.py:
 client._connect_scheduler = mock.MagicMock()
 src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
 Mock()
 src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  
 job.assignedTask.task.executorConfig.data = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = 
 Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = 
 Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock()
 src/test/python/apache/aurora/client/commands/test_diff.py:  job = Mock()
 src/test/python/apache/aurora/client/commands/test_diff.py:  
 job.assignedTask.task.executorConfig.data = Mock()
 src/test/python/apache/aurora/client/commands/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/commands/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/commands/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/commands/test_listjobs.py:
 mock_options = Mock()
 src/test/python/apache/aurora/client/commands/test_listjobs.py:  job = 
 Mock()
 src/test/python/apache/aurora/client/commands/test_maintenance.py:
 mock_callback = Mock()
 src/test/python/apache/aurora/client/commands/test_maintenance.py:  
 mock_wait = Mock()
 src/test/python/apache/aurora/client/commands/util.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = 
 Mock()
 
 
 

Re: Review Request 27084: Fix error when job create is called with --open-browser.

2014-10-24 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Oct. 23, 2014, 9:50 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27084/
 ---
 
 (Updated Oct. 23, 2014, 9:50 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-886
 https://issues.apache.org/jira/browse/aurora-886
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error when job create is called with --open-browser.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/jobs.py 
 10f8e0d331ca607e55e3aa6f96014caea744ed9f 
   src/test/python/apache/aurora/client/cli/test_create.py 
 328297ab1d29efb0adce8f4931a13929a04dcd9c 
 
 Diff: https://reviews.apache.org/r/27084/diff/
 
 
 Testing
 ---
 
 Added new test; all unit tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Zameer Manji


 On Oct. 24, 2014, 2:10 p.m., Maxim Khutornenko wrote:
  It's not obvious from the diff: where do we gain perf? Can you point to the 
  place where we don't do store calls anymore?

If you notice in `SchedulingFilterImplTest.java` many of the tests no longer 
make calls to the maintenance controller. This is a performance gain because 
the maintenance controller makes a consistent read from the AttributeStore. 
Since this read is ommitted from every time we call the scheduling filter we 
gain performance.


- Zameer


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


On Oct. 24, 2014, 12:59 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 12:59 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Joshua Cohen


 On Oct. 24, 2014, 5:57 p.m., Joshua Cohen wrote:
  Does it make sense to use the ReviewBoard Python client from rbtools? 
  https://www.reviewboard.org/docs/rbtools/0.5/api/overview/
  
  What is the plan to actually run this script? Is it safe to assume that it 
  will be executed from an up to date git repo, or will it need to git pull?
 
 Bill Farner wrote:
 I'm hesitant, mostly because i don't think the code savings is worth the 
 cost of adding build infrastructure around this script.  I could be swayed, 
 though.

Yeah, I'm not sure it's worth it either, but figured it's worth mentioning.


- Joshua


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


On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 5:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Bill Farner


 On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote:
  build-support/jenkins/review_feedback.py, line 39
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line39
 
  Use print() as a function, here and throughout

Done.


 On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote:
  build-support/jenkins/review_feedback.py, line 94
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line94
 
  as we're on python2.7 you can use argparse now (which removes some 
  boilerplate and gives nicer errors)

Thanks, done.


 On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote:
  build-support/jenkins/review_feedback.py, line 97
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line97
 
  read this from a file? argparse makes this easy

Sure, done.


 On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote:
  build-support/jenkins/review_feedback.py, line 116
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line116
 
  with argparse you can just do required=True

Done, thanks again.


 On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote:
  build-support/jenkins/review_feedback.py, line 149
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line149
 
  Would the correct Apache exclaimation be +1 here?

Sure, done.


- Bill


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


On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 5:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Bill Farner


 On Oct. 24, 2014, 8:57 p.m., Maxim Khutornenko wrote:
  build-support/jenkins/review_feedback.py, lines 37-38
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line37
 
  replace with ternary?

Done.


 On Oct. 24, 2014, 8:57 p.m., Maxim Khutornenko wrote:
  build-support/jenkins/review_feedback.py, line 45
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line45
 
  Is there a legitimate case to allow non 200 response code here?

201 for POST.


 On Oct. 24, 2014, 8:57 p.m., Maxim Khutornenko wrote:
  build-support/jenkins/review_feedback.py, line 47
  https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line47
 
  Combine with the above print?

Done.


- Bill


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


On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 5:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Bill Farner

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

(Updated Oct. 24, 2014, 9:42 p.m.)


Review request for Aurora, Kevin Sweeney and Zameer Manji.


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


Repository: aurora


Description
---

I also removed `--stacktrace` from the gradle command.  This makes for better 
tail output.  I originally added `--stacktrace` to help debug build flakiness 
we were having due to jenkins machine configuration (but haven't had in a very 
long time): https://reviews.apache.org/r/23776/


Diffs (updated)
-

  build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
  build-support/jenkins/review_feedback.py PRE-CREATION 

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


Testing
---

I've run this a handful of times on my workstation, it seems to be doing its 
job.


Thanks,

Bill Farner



Re: Review Request 27084: Fix error when job create is called with --open-browser.

2014-10-24 Thread David McLaughlin

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

Ship it!


Ship It!

- David McLaughlin


On Oct. 23, 2014, 4:50 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27084/
 ---
 
 (Updated Oct. 23, 2014, 4:50 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-886
 https://issues.apache.org/jira/browse/aurora-886
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error when job create is called with --open-browser.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/jobs.py 
 10f8e0d331ca607e55e3aa6f96014caea744ed9f 
   src/test/python/apache/aurora/client/cli/test_create.py 
 328297ab1d29efb0adce8f4931a13929a04dcd9c 
 
 Diff: https://reviews.apache.org/r/27084/diff/
 
 
 Testing
 ---
 
 Added new test; all unit tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Zameer Manji

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

Ship it!


Can you document your exit codes somewhere?

- Zameer Manji


On Oct. 24, 2014, 2:42 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 2:42 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Aurora ReviewBot

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


+1: Master (5be667f) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 24, 2014, 9:42 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 9:42 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Oct. 24, 2014, 9:42 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 9:42 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Aurora ReviewBot

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


+1: Master (5be667f) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 7:59 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread Aurora ReviewBot

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


-1: Master (5be667f) is red with this patch.
  ./build-support/jenkins/build.sh

src.test.python.apache.aurora.client.api.updater_util   
.   SUCCESS
src.test.python.apache.aurora.client.binding_helper 
.   SUCCESS
src.test.python.apache.aurora.client.cli.api
.   SUCCESS
src.test.python.apache.aurora.client.cli.bridge 
.   SUCCESS
src.test.python.apache.aurora.client.cli.command_hooks  
.   SUCCESS
src.test.python.apache.aurora.client.cli.config 
.   SUCCESS
src.test.python.apache.aurora.client.cli.cron   
.   SUCCESS
src.test.python.apache.aurora.client.cli.help   
.   SUCCESS
src.test.python.apache.aurora.client.cli.inspect
.   SUCCESS
src.test.python.apache.aurora.client.cli.job
.   SUCCESS
src.test.python.apache.aurora.client.cli.logging
.   SUCCESS
src.test.python.apache.aurora.client.cli.plugins
.   SUCCESS
src.test.python.apache.aurora.client.cli.quota  
.   SUCCESS
src.test.python.apache.aurora.client.cli.sla
.   SUCCESS
src.test.python.apache.aurora.client.cli.supdate
.   SUCCESS
src.test.python.apache.aurora.client.cli.task   
.   SUCCESS
src.test.python.apache.aurora.client.cli.update 
.   SUCCESS
src.test.python.apache.aurora.client.commands.admin 
.   SUCCESS
src.test.python.apache.aurora.client.commands.core  
.   FAILURE
src.test.python.apache.aurora.client.config 
.   SUCCESS

- Aurora ReviewBot


On Oct. 24, 2014, 8:09 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27058/
 ---
 
 (Updated Oct. 24, 2014, 8:09 p.m.)
 
 
 Review request for Aurora, Mark Chu-Carroll and Zameer Manji.
 
 
 Bugs: AURORA-248
 https://issues.apache.org/jira/browse/AURORA-248
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use of Mock() without a specification considered harmful. I went through and 
 updated as many mocks as I could. 
 
 Any remaining can be classified as:
 
 1) Mocks of classes that cannot be spec'd. Almost all instances of 
 SchedulerProxy, which uses __getattr__ to delegate to the read or write 
 client. 
 2) Primitives like strings and callback functions or data objects like dicts 
 and pystachio structs.
 3) Weird mocks that broke code where they really shouldn't have (off the top 
 of my head - in test_diff.py and commands/test_run.py) - both when they were 
 spec'd and when they were replaced with real thrift structs. 
 
 
 The remaining offenders:
 
 $ grep -r --include=*.py Mock() src/test/python 
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
 Mock()
 src/test/python/apache/aurora/client/api/test_job_monitor.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_quota_check.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_scheduler_client.py:
 client._connect_scheduler = mock.MagicMock()
 src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
 Mock()
 src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  
 job.assignedTask.task.executorConfig.data = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 

Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Bill Farner


 On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote:
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java, 
  line 119
  https://reviews.apache.org/r/27100/diff/1/?file=730205#file730205line119
 
  I don't understand what you mean by matches what we do for dedicated 
  host mismatches as well.

Here's the code snippet:

if (!ConfigurationManager.isDedicated(task)  isDedicated(slaveHost)) {
  return ImmutableSet.of(DEDICATED_HOST_VETO);
}
return ImmutableSet.Vetobuilder()
.addAll(getConstraintFilter(attributeAggregate, slaveHost).apply(task))
.addAll(getResourceVetoes(offer, task))
.addAll(getMaintenanceVeto(slaveHost).asSet())
.build();

If the host is a known completely bad fit, we don't look at it further.  I 
believe the same flow could be done for maintenance veteos, but at the caller 
rather than in `SchedulingFilterImpl`.


- Bill


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


On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 7:59 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27114: Move from github to bintray for pants support binaries.

2014-10-24 Thread Bill Farner

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

Ship it!


Thanks!  This is now on master:

$ git log -1 origin/master --abbrev-commit
commit f98bec7
Author: Joe Smith yasumo...@gmail.com
Date:   Fri Oct 24 15:13:21 2014 -0700

Move from github to bintray for pants support binaries.

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

- Bill Farner


On Oct. 23, 2014, 10:19 p.m., Joe Smith wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27114/
 ---
 
 (Updated Oct. 23, 2014, 10:19 p.m.)
 
 
 Review request for Aurora, John Sirois, Kevin Sweeney, and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Move from github to bintray for pants support binaries.
 
 This is from advice from jsirois on 
 https://groups.google.com/d/msg/pants-devel/khP3TuSqWmo/nLqgc671Og0J
 
 
 Diffs
 -
 
   pants.ini 6f49f94236d1e1ed721578365173c96dc4420b02 
 
 Diff: https://reviews.apache.org/r/27114/diff/
 
 
 Testing
 ---
 
 [tw-mbp13-jsmith aurora (yasumoto/pants_bintray)]$ ./pants 
 ./src/test/python/apache/aurora/admin:all
 Build operating on top level addresses: 
 set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/admin/BUILD,
  all)])
 
  test session starts 
 =
 platform darwin -- Python 2.7.6 -- py-1.4.25 -- pytest-2.6.3
 plugins: cov, timeout
 collected 11 items 
 
 src/test/python/apache/aurora/admin/test_host_maintenance.py ...
 
 =
  11 passed in 0.62 seconds 
 ==
 
  test session starts 
 =
 platform darwin -- Python 2.7.6 -- py-1.4.25 -- pytest-2.6.3
 plugins: cov, timeout
 collected 2 items 
 
 src/test/python/apache/aurora/admin/test_admin_util.py ..
 
 ==
  2 passed in 0.19 seconds 
 ==
 src.test.python.apache.aurora.admin.admin_util
   .   SUCCESS
 src.test.python.apache.aurora.admin.host_maintenance  
   .   SUCCESS
 
 
 Thanks,
 
 Joe Smith
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Zameer Manji

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

(Updated Oct. 24, 2014, 3:38 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Bill's feedback. The approach is different than suggested after talking with 
Bill offline.


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


Repository: aurora


Description
---

Cache the host's maintenance status with the offer. By caching the status
alongside the offer the scheduler does not need to access the attribute store
for every offer it considers for a task.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
92c843830df7a779abace38bb0ce84d4cbeb5af4 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
4088559c39014befaddb0b29dad45fac9f4545c4 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
da29428adaebcb27b20a10a8c6b7e380662fce4a 
  src/main/java/org/apache/aurora/scheduler/http/Offers.java 
b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
4db9be86f2e7db08d12e0182914a7c5130301b13 
  src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
  src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
  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/events/NotifyingSchedulingFilterTest.java
 4065629e9d488b122aa811b9802def0b51a21294 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
6a9c4ee278ed3ee8222404504e571f20991c2ae2 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
c48cbae4864127e7799917182439f7670285b0d3 

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


Testing
---

./gradlew clean build -Pq


Thanks,

Zameer Manji



Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Zameer Manji


 On Oct. 24, 2014, 2:49 p.m., Zameer Manji wrote:
  Can you document your exit codes somewhere?
 
 Bill Farner wrote:
 Is that worthwhile?  If it means documenting, i'd rather not vary them at 
 all.  I don't think this is something that should be scripted against.

I asumed you varried them for a reason. If you are not scripting against them 
then just do sys.exit(1) otherwise it is confusing IMHO.


- Zameer


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


On Oct. 24, 2014, 2:42 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 2:42 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Bill Farner

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

Ship it!



src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
https://reviews.apache.org/r/27100/#comment99410

// TODO(wfarner): Replace this with HostAttributes for more use of this 
caching.



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java
https://reviews.apache.org/r/27100/#comment99412

maintenanceVeto.isPresent()


- Bill Farner


On Oct. 24, 2014, 10:38 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 10:38 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Bill Farner


 On Oct. 24, 2014, 9:49 p.m., Zameer Manji wrote:
  Can you document your exit codes somewhere?
 
 Bill Farner wrote:
 Is that worthwhile?  If it means documenting, i'd rather not vary them at 
 all.  I don't think this is something that should be scripted against.
 
 Zameer Manji wrote:
 I asumed you varried them for a reason. If you are not scripting against 
 them then just do sys.exit(1) otherwise it is confusing IMHO.

Agreed, done.


- Bill


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


On Oct. 24, 2014, 9:42 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 9:42 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Bill Farner

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

(Updated Oct. 24, 2014, 10:42 p.m.)


Review request for Aurora, Kevin Sweeney and Zameer Manji.


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


Repository: aurora


Description
---

I also removed `--stacktrace` from the gradle command.  This makes for better 
tail output.  I originally added `--stacktrace` to help debug build flakiness 
we were having due to jenkins machine configuration (but haven't had in a very 
long time): https://reviews.apache.org/r/23776/


Diffs (updated)
-

  build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
  build-support/jenkins/review_feedback.py PRE-CREATION 

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


Testing
---

I've run this a handful of times on my workstation, it seems to be doing its 
job.


Thanks,

Bill Farner



Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread David McLaughlin

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

(Updated Oct. 24, 2014, 10:48 p.m.)


Review request for Aurora, Mark Chu-Carroll and Zameer Manji.


Changes
---

Thanks ReviewBot! I forgot to run the commands target rather than just the cli 
one. 


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


Repository: aurora


Description
---

Use of Mock() without a specification considered harmful. I went through and 
updated as many mocks as I could. 

Any remaining can be classified as:

1) Mocks of classes that cannot be spec'd. Almost all instances of 
SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 
2) Primitives like strings and callback functions or data objects like dicts 
and pystachio structs.
3) Weird mocks that broke code where they really shouldn't have (off the top of 
my head - in test_diff.py and commands/test_run.py) - both when they were 
spec'd and when they were replaced with real thrift structs. 


The remaining offenders:

$ grep -r --include=*.py Mock() src/test/python 
src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback 
= mock.Mock()
src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback 
= mock.Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
Mock()
src/test/python/apache/aurora/client/api/test_job_monitor.py:
self._scheduler = Mock()
src/test/python/apache/aurora/client/api/test_quota_check.py:
self._scheduler = Mock()
src/test/python/apache/aurora/client/api/test_scheduler_client.py:
client._connect_scheduler = mock.MagicMock()
src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
Mock()
src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:  
job.assignedTask.task.executorConfig.data = Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock()
src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock()
src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = 
Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = 
Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = 
Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:  job = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:  
job.assignedTask.task.executorConfig.data = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_listjobs.py:mock_options 
= Mock()
src/test/python/apache/aurora/client/commands/test_listjobs.py:  job = 
Mock()
src/test/python/apache/aurora/client/commands/test_maintenance.py:
mock_callback = Mock()
src/test/python/apache/aurora/client/commands/test_maintenance.py:  
mock_wait = Mock()
src/test/python/apache/aurora/client/commands/util.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = 
Mock()


Diffs (updated)
-

  src/test/python/apache/aurora/admin/test_admin_util.py 
f5c8c69c1109d15ee3886fb863014c3285240db1 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
60c75300501c36ac20a97f78ff18b3ca7af30699 
  src/test/python/apache/aurora/client/cli/test_cron.py 
c7b71c29d44150162fec8066947623fa91815424 
  

Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread Kevin Sweeney

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

Ship it!


Long live ReviewBot!

- Kevin Sweeney


On Oct. 24, 2014, 3:48 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27058/
 ---
 
 (Updated Oct. 24, 2014, 3:48 p.m.)
 
 
 Review request for Aurora, Mark Chu-Carroll and Zameer Manji.
 
 
 Bugs: AURORA-248
 https://issues.apache.org/jira/browse/AURORA-248
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use of Mock() without a specification considered harmful. I went through and 
 updated as many mocks as I could. 
 
 Any remaining can be classified as:
 
 1) Mocks of classes that cannot be spec'd. Almost all instances of 
 SchedulerProxy, which uses __getattr__ to delegate to the read or write 
 client. 
 2) Primitives like strings and callback functions or data objects like dicts 
 and pystachio structs.
 3) Weird mocks that broke code where they really shouldn't have (off the top 
 of my head - in test_diff.py and commands/test_run.py) - both when they were 
 spec'd and when they were replaced with real thrift structs. 
 
 
 The remaining offenders:
 
 $ grep -r --include=*.py Mock() src/test/python 
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
 Mock()
 src/test/python/apache/aurora/client/api/test_job_monitor.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_quota_check.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_scheduler_client.py:
 client._connect_scheduler = mock.MagicMock()
 src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
 Mock()
 src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  
 job.assignedTask.task.executorConfig.data = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = 
 Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = 
 Mock()
 src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock()
 src/test/python/apache/aurora/client/commands/test_diff.py:  job = Mock()
 src/test/python/apache/aurora/client/commands/test_diff.py:  
 job.assignedTask.task.executorConfig.data = Mock()
 src/test/python/apache/aurora/client/commands/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/commands/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/commands/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/commands/test_listjobs.py:
 mock_options = Mock()
 src/test/python/apache/aurora/client/commands/test_listjobs.py:  job = 
 Mock()
 src/test/python/apache/aurora/client/commands/test_maintenance.py:
 mock_callback = Mock()
 src/test/python/apache/aurora/client/commands/test_maintenance.py:  
 mock_wait = Mock()
 src/test/python/apache/aurora/client/commands/util.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = 
 

Re: Review Request 27145: Add a script that publishes build results to review board.

2014-10-24 Thread Aurora ReviewBot

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


+1: Master (f98bec7) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 24, 2014, 10:42 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27145/
 ---
 
 (Updated Oct. 24, 2014, 10:42 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-883
 https://issues.apache.org/jira/browse/AURORA-883
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I also removed `--stacktrace` from the gradle command.  This makes for better 
 tail output.  I originally added `--stacktrace` to help debug build flakiness 
 we were having due to jenkins machine configuration (but haven't had in a 
 very long time): https://reviews.apache.org/r/23776/
 
 
 Diffs
 -
 
   build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 
   build-support/jenkins/review_feedback.py PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27145/diff/
 
 
 Testing
 ---
 
 I've run this a handful of times on my workstation, it seems to be doing its 
 job.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Oct. 24, 2014, 10:38 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 10:38 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27089: Add documentation for the beta-update command.

2014-10-24 Thread Bill Farner

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

(Updated Oct. 24, 2014, 11:05 p.m.)


Review request for Aurora, David McLaughlin and Maxim Khutornenko.


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


Repository: aurora


Description
---

Add documentation for the beta-update command.


Diffs
-

  docs/client-commands.md 0424e8f5b386e0342a4c06870bbcbc01c0f20323 

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


Testing
---

Rendered at 
https://github.com/wfarner/incubator-aurora/blob/wfarner/document_updater/docs/client-commands.md


Thanks,

Bill Farner



Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Zameer Manji

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

(Updated Oct. 24, 2014, 4:08 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Bill's feedback.


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


Repository: aurora


Description
---

Cache the host's maintenance status with the offer. By caching the status
alongside the offer the scheduler does not need to access the attribute store
for every offer it considers for a task.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
92c843830df7a779abace38bb0ce84d4cbeb5af4 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
4088559c39014befaddb0b29dad45fac9f4545c4 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
da29428adaebcb27b20a10a8c6b7e380662fce4a 
  src/main/java/org/apache/aurora/scheduler/http/Offers.java 
b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
4db9be86f2e7db08d12e0182914a7c5130301b13 
  src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
  src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
  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/events/NotifyingSchedulingFilterTest.java
 4065629e9d488b122aa811b9802def0b51a21294 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
6a9c4ee278ed3ee8222404504e571f20991c2ae2 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
c48cbae4864127e7799917182439f7670285b0d3 

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


Testing
---

./gradlew clean build -Pq


Thanks,

Zameer Manji



Re: Review Request 27100: Cache the host's maintenance status with offer.

2014-10-24 Thread Aurora ReviewBot

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


+1: Master (669981d) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 24, 2014, 11:08 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27100/
 ---
 
 (Updated Oct. 24, 2014, 11:08 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-878
 https://issues.apache.org/jira/browse/AURORA-878
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cache the host's maintenance status with the offer. By caching the status
 alongside the offer the scheduler does not need to access the attribute store
 for every offer it considers for a task.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
 92c843830df7a779abace38bb0ce84d4cbeb5af4 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 4088559c39014befaddb0b29dad45fac9f4545c4 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 882cdfd9b79b262befb81437cbd9a31a6bc1e40f 
   
 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
  5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 
   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
 da29428adaebcb27b20a10a8c6b7e380662fce4a 
   src/main/java/org/apache/aurora/scheduler/http/Offers.java 
 b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 
   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
 4db9be86f2e7db08d12e0182914a7c5130301b13 
   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
 c4435cb74925c9ed04a37820b22c3ecdfcad49d4 
   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 
 ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
   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/events/NotifyingSchedulingFilterTest.java
  4065629e9d488b122aa811b9802def0b51a21294 
   
 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
  6a9c4ee278ed3ee8222404504e571f20991c2ae2 
   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
 c48cbae4864127e7799917182439f7670285b0d3 
 
 Diff: https://reviews.apache.org/r/27100/diff/
 
 
 Testing
 ---
 
 ./gradlew clean build -Pq
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 27089: Add documentation for the beta-update command.

2014-10-24 Thread Maxim Khutornenko

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

Ship it!



docs/client-commands.md
https://reviews.apache.org/r/27089/#comment99430

Perhaps briefly mention the benefits of the server side updater and the 
upcoming removal of the client one?



docs/client-commands.md
https://reviews.apache.org/r/27089/#comment99427

missing closing ''


- Maxim Khutornenko


On Oct. 24, 2014, 11:05 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27089/
 ---
 
 (Updated Oct. 24, 2014, 11:05 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Bugs: AURORA-763
 https://issues.apache.org/jira/browse/AURORA-763
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add documentation for the beta-update command.
 
 
 Diffs
 -
 
   docs/client-commands.md 0424e8f5b386e0342a4c06870bbcbc01c0f20323 
 
 Diff: https://reviews.apache.org/r/27089/diff/
 
 
 Testing
 ---
 
 Rendered at 
 https://github.com/wfarner/incubator-aurora/blob/wfarner/document_updater/docs/client-commands.md
 
 
 Thanks,
 
 Bill Farner
 




Review Request 27181: Allow the invoker to ignore patterns in git-clean.

2014-10-24 Thread Bill Farner

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

Review request for Aurora and Kevin Sweeney.


Repository: aurora


Description
---

Like, i dunno, the credentials file, for example.


Diffs
-

  build-support/jenkins/review_feedback.py 
049f2ceec4acc0a6901105d7c5fcccbbdbb90712 

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


Testing
---

Running.


Thanks,

Bill Farner



Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.

2014-10-24 Thread Kevin Sweeney

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

Ship it!



build-support/jenkins/review_feedback.py
https://reviews.apache.org/r/27181/#comment99437

Should've caught this in the first review - dashes instead of underscores, 
here and above.


- Kevin Sweeney


On Oct. 24, 2014, 4:19 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27181/
 ---
 
 (Updated Oct. 24, 2014, 4:19 p.m.)
 
 
 Review request for Aurora and Kevin Sweeney.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Like, i dunno, the credentials file, for example.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 
 
 Diff: https://reviews.apache.org/r/27181/diff/
 
 
 Testing
 ---
 
 Running.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.

2014-10-24 Thread Bill Farner


 On Oct. 24, 2014, 11:21 p.m., Kevin Sweeney wrote:
  build-support/jenkins/review_feedback.py, line 112
  https://reviews.apache.org/r/27181/diff/1/?file=733134#file733134line112
 
  Should've caught this in the first review - dashes instead of 
  underscores, here and above.

Done.


- Bill


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


On Oct. 24, 2014, 11:19 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27181/
 ---
 
 (Updated Oct. 24, 2014, 11:19 p.m.)
 
 
 Review request for Aurora and Kevin Sweeney.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Like, i dunno, the credentials file, for example.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 
 
 Diff: https://reviews.apache.org/r/27181/diff/
 
 
 Testing
 ---
 
 Running.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.

2014-10-24 Thread Aurora ReviewBot

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


-1: This patch does not apply cleanly on master (f98bec7), do you need to 
rebase?

- Aurora ReviewBot


On Oct. 24, 2014, 11:19 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27181/
 ---
 
 (Updated Oct. 24, 2014, 11:19 p.m.)
 
 
 Review request for Aurora and Kevin Sweeney.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Like, i dunno, the credentials file, for example.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 
 
 Diff: https://reviews.apache.org/r/27181/diff/
 
 
 Testing
 ---
 
 Running.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.

2014-10-24 Thread Bill Farner

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

(Updated Oct. 24, 2014, 11:25 p.m.)


Review request for Aurora and Kevin Sweeney.


Repository: aurora


Description
---

Like, i dunno, the credentials file, for example.


Diffs (updated)
-

  build-support/jenkins/review_feedback.py 
049f2ceec4acc0a6901105d7c5fcccbbdbb90712 

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


Testing
---

Running.


Thanks,

Bill Farner



Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread Aurora ReviewBot

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


-1: Master (3778330) is red with this patch.
  ./build-support/jenkins/build.sh

Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)

Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.lang-0.3.0-py2.7-nspkg.pth
Successfully installed twitter.checkstyle pyflakes pep8 GitPython 
twitter.common.app gitdb twitter.common.process twitter.common.log 
twitter.common.util twitter.common.collections async smmap 
twitter.common.string twitter.common.options twitter.common.dirutil 
twitter.common.contextutil twitter.common.lang
Cleaning up...
F401:ERROR   src/test/python/apache/aurora/client/cli/test_task_run.py:024-031 
'ScheduledTask' imported but unused
 |from gen.apache.aurora.api.ttypes import (
 |JobKey,
 |ResponseCode,
 |ScheduledTask,
 |ScheduleStatus,
 |ScheduleStatusResult,
 |TaskQuery
 |)

E501:ERROR   
src/test/python/apache/aurora/client/commands/test_maintenance.py:045-046 line 
too long (101  100 characters)
 |mock_options = Mock(spec_set=['cluster', 'disable_all_hooks', 
'duration', 'filename', 'grouping',
 |  'hosts', 'percentage', 'post_drain_script', 'reason', 
'unsafe_hosts_filename', 'verbosity'])

- Aurora ReviewBot


On Oct. 24, 2014, 10:48 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27058/
 ---
 
 (Updated Oct. 24, 2014, 10:48 p.m.)
 
 
 Review request for Aurora, Mark Chu-Carroll and Zameer Manji.
 
 
 Bugs: AURORA-248
 https://issues.apache.org/jira/browse/AURORA-248
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use of Mock() without a specification considered harmful. I went through and 
 updated as many mocks as I could. 
 
 Any remaining can be classified as:
 
 1) Mocks of classes that cannot be spec'd. Almost all instances of 
 SchedulerProxy, which uses __getattr__ to delegate to the read or write 
 client. 
 2) Primitives like strings and callback functions or data objects like dicts 
 and pystachio structs.
 3) Weird mocks that broke code where they really shouldn't have (off the top 
 of my head - in test_diff.py and commands/test_run.py) - both when they were 
 spec'd and when they were replaced with real thrift structs. 
 
 
 The remaining offenders:
 
 $ grep -r --include=*.py Mock() src/test/python 
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/admin/test_host_maintenance.py:
 mock_callback = mock.Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
 src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
 Mock()
 src/test/python/apache/aurora/client/api/test_job_monitor.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_quota_check.py:
 self._scheduler = Mock()
 src/test/python/apache/aurora/client/api/test_scheduler_client.py:
 client._connect_scheduler = mock.MagicMock()
 src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
 Mock()
 src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:  
 job.assignedTask.task.executorConfig.data = Mock()
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_diff.py:
 patch('json.loads', return_value=Mock())) as (
 src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = 
 Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 src/test/python/apache/aurora/client/cli/test_kill.py:
 mock_scheduler_proxy = Mock()
 

Review Request 27182: Add a test for the thermos resource module

2014-10-24 Thread Joe Smith

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

Review request for Aurora and Kevin Sweeney.


Repository: aurora


Description
---

Add a test for the thermos resource module


Diffs
-

  src/main/python/apache/thermos/monitoring/monitor.py 
8f87f5ffc39c87e87ff78b941ea30df7138bd1ef 
  src/test/python/apache/thermos/monitoring/BUILD 
33d6bba43aff6d62b2646491f004475c27ed99db 
  src/test/python/apache/thermos/monitoring/test_resource.py PRE-CREATION 

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


Testing
---

[tw-mbp13-jsmith aurora (yasumoto/psutil_2.1.3)]$ ./pants 
./src/test/python/apache/thermos/monitoring:test_resource
Build operating on top level addresses: 
set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/thermos/monitoring/BUILD,
 test_resource)])

 test session starts 
=
platform darwin -- Python 2.7.6 -- py-1.4.26 -- pytest-2.6.4
plugins: cov, timeout
collected 5 items 

src/test/python/apache/thermos/monitoring/test_resource.py .

==
 5 passed in 0.21 seconds 
==
src.test.python.apache.thermos.monitoring.test_resource 
.   SUCCESS


Thanks,

Joe Smith



Re: Review Request 27058: Add specs to instances of Mock in Python tests.

2014-10-24 Thread David McLaughlin

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

(Updated Oct. 25, 2014, 12:24 a.m.)


Review request for Aurora, Mark Chu-Carroll and Zameer Manji.


Changes
---

Okay ran _all_ tests, and also ran isort-check and checkstyle-check again.


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


Repository: aurora


Description
---

Use of Mock() without a specification considered harmful. I went through and 
updated as many mocks as I could. 

Any remaining can be classified as:

1) Mocks of classes that cannot be spec'd. Almost all instances of 
SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 
2) Primitives like strings and callback functions or data objects like dicts 
and pystachio structs.
3) Weird mocks that broke code where they really shouldn't have (off the top of 
my head - in test_diff.py and commands/test_run.py) - both when they were 
spec'd and when they were replaced with real thrift structs. 


The remaining offenders:

$ grep -r --include=*.py Mock() src/test/python 
src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback 
= mock.Mock()
src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback 
= mock.Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock()
src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = 
Mock()
src/test/python/apache/aurora/client/api/test_job_monitor.py:
self._scheduler = Mock()
src/test/python/apache/aurora/client/api/test_quota_check.py:
self._scheduler = Mock()
src/test/python/apache/aurora/client/api/test_scheduler_client.py:
client._connect_scheduler = mock.MagicMock()
src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = 
Mock()
src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = 
Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:  job = Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:  
job.assignedTask.task.executorConfig.data = Mock()
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _):
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/cli/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock()
src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock()
src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = 
Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = 
Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = 
Mock()
src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:  job = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:  
job.assignedTask.task.executorConfig.data = Mock()
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_diff.py:
patch('json.loads', return_value=Mock())) as (
src/test/python/apache/aurora/client/commands/test_listjobs.py:mock_options 
= Mock()
src/test/python/apache/aurora/client/commands/test_listjobs.py:  job = 
Mock()
src/test/python/apache/aurora/client/commands/test_maintenance.py:
mock_callback = Mock()
src/test/python/apache/aurora/client/commands/test_maintenance.py:  
mock_wait = Mock()
src/test/python/apache/aurora/client/commands/util.py:mock_scheduler_proxy 
= Mock()
src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = 
Mock()


Diffs (updated)
-

  src/test/python/apache/aurora/admin/test_admin_util.py 
f5c8c69c1109d15ee3886fb863014c3285240db1 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
60c75300501c36ac20a97f78ff18b3ca7af30699 
  src/test/python/apache/aurora/client/cli/test_cron.py 
c7b71c29d44150162fec8066947623fa91815424 
  

Review Request 27188: Use ship-it instead of +/-1.

2014-10-24 Thread Bill Farner

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

Review request for Aurora and Jake Farrell.


Repository: aurora


Description
---

Use ship-it instead of +/-1.


Diffs
-

  build-support/jenkins/review_feedback.py 
9d22358608d0d52019df0c4c5b96b08d0f157c43 

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


Testing
---

None yet, will do a dry run Monday before committing.


Thanks,

Bill Farner



Re: Review Request 27188: Use ship-it instead of +/-1.

2014-10-24 Thread Joe Smith

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

Ship it!


Ship It!

- Joe Smith


On Oct. 24, 2014, 8:17 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27188/
 ---
 
 (Updated Oct. 24, 2014, 8:17 p.m.)
 
 
 Review request for Aurora and Jake Farrell.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use ship-it instead of +/-1.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 9d22358608d0d52019df0c4c5b96b08d0f157c43 
 
 Diff: https://reviews.apache.org/r/27188/diff/
 
 
 Testing
 ---
 
 None yet, will do a dry run Monday before committing.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27188: Use ship-it instead of +/-1.

2014-10-24 Thread Aurora ReviewBot

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


+1: Master (3778330) is green with this patch.
  ./build-support/jenkins/build.sh

- Aurora ReviewBot


On Oct. 25, 2014, 3:17 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27188/
 ---
 
 (Updated Oct. 25, 2014, 3:17 a.m.)
 
 
 Review request for Aurora and Jake Farrell.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use ship-it instead of +/-1.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 9d22358608d0d52019df0c4c5b96b08d0f157c43 
 
 Diff: https://reviews.apache.org/r/27188/diff/
 
 
 Testing
 ---
 
 None yet, will do a dry run Monday before committing.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27188: Use ship-it instead of +/-1.

2014-10-24 Thread Joshua Cohen

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

Ship it!


_~
   _~)_)_~
  )_))_))_)
  _!__!__!_
  __t/
~

- Joshua Cohen


On Oct. 25, 2014, 3:17 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27188/
 ---
 
 (Updated Oct. 25, 2014, 3:17 a.m.)
 
 
 Review request for Aurora and Jake Farrell.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use ship-it instead of +/-1.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 9d22358608d0d52019df0c4c5b96b08d0f157c43 
 
 Diff: https://reviews.apache.org/r/27188/diff/
 
 
 Testing
 ---
 
 None yet, will do a dry run Monday before committing.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 27188: Use ship-it instead of +/-1.

2014-10-24 Thread Joshua Cohen


 On Oct. 25, 2014, 4:39 a.m., Joshua Cohen wrote:
  _~
 _~)_)_~
)_))_))_)
_!__!__!_
__t/
  ~

Wow did that ever not format properly!

https://github.com/reviewboard/rb-extension-pack/blob/master/shipit_ascii_art/shipit_ascii_art/asciiart.py#L4


- Joshua


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


On Oct. 25, 2014, 3:17 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27188/
 ---
 
 (Updated Oct. 25, 2014, 3:17 a.m.)
 
 
 Review request for Aurora and Jake Farrell.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use ship-it instead of +/-1.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 9d22358608d0d52019df0c4c5b96b08d0f157c43 
 
 Diff: https://reviews.apache.org/r/27188/diff/
 
 
 Testing
 ---
 
 None yet, will do a dry run Monday before committing.
 
 
 Thanks,
 
 Bill Farner