Re: Review Request 26997: Adding quota check into scheduleCronJob RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26997/ --- (Updated Oct. 22, 2014, 4 p.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Changes --- CR comments. Bugs: AURORA-824 https://issues.apache.org/jira/browse/AURORA-824 Repository: aurora Description --- Adding quota check into scheduleCronJob. Also, added missing tests for cron-related RPCs. Diffs (updated) - build.gradle 459c79d1842bb6ee73e6a4ce8fd17b31113c3e79 src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java 48b403e155684861ec0307a61ccb0775cdb57a32 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java e792d23d6bb13b4e61b078beea6d063f72f0d8fc src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f36a88fda3539553800bd727c3d2a77a54f1e71c Diff: https://reviews.apache.org/r/26997/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 26998: Re-building Aurora components before running e2e tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26998/ --- (Updated Oct. 22, 2014, 4:04 p.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Summary (updated) - Re-building Aurora components before running e2e tests. Bugs: AURORA-498 https://issues.apache.org/jira/browse/AURORA-498 Repository: aurora Description --- Building aurora client/admin before running e2 tests. Diffs - examples/vagrant/aurorabuild.sh 8659bffb8fb6170c02aef0edce92349540d4366a examples/vagrant/provision-dev-cluster.sh 1d4fd77a83dbfc6724a3a3b5f44301dc54b3085c src/test/sh/org/apache/aurora/e2e/test_common.sh 43d2516133c6d6cdb4236358f942396f057f739c src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 324aa4dbeff00e673fe73b87e3a0766856cd213c src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh bbbf90b95e91bcdf8aaf8b2a7b577dee70a7c8a7 Diff: https://reviews.apache.org/r/26998/diff/ Testing --- ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh Thanks, Maxim Khutornenko
Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 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/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 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/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Review Request 27045: Add some wiggle room before requiring min coverage thresholds be raised.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27045/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-880 https://issues.apache.org/jira/browse/AURORA-880 Repository: aurora Description --- Add some wiggle room before requiring min coverage thresholds be raised. Diffs - build.gradle 62d1492215313d6619491703fc88da3010f60886 buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 16f648b412b4d42c83c83da520358167b034ee25 Diff: https://reviews.apache.org/r/27045/diff/ Testing --- $ ./gradlew clean build [...] :jacocoTestReport Coverage report generated: file:///Users/jcohen/workspace/external/incubator-aurora/dist/reports/jacoco/test/html/index.html :analyzeReport Instruction coverage of 0.8806389838314789 exceeds minimum coverage of 0.87. Branch coverage of 0.825925925925926 exceeds minimum coverage of 0.82. :check :build BUILD SUCCESSFUL Thanks, Joshua Cohen
Re: Review Request 27044: Make executor overhead configurable
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27044/#review57825
---
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
https://reviews.apache.org/r/27044/#comment98695
should this be a static import?
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java
https://reviews.apache.org/r/27044/#comment98697
Continunation indent style is for these to be indented 4 spaces past the
parent, not lined up.
So:
public SchedulingFilterImpl(
Storage storage,
MaintenanceController maintenance,
...) {
// code
}
src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/27044/#comment98698
Do we actually need to use a ResourceSlotFactory here? The usage seems to
be entirely internal to the test. We could just use constants?
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
https://reviews.apache.org/r/27044/#comment98699
Indentation looks off here?
- Joshua Cohen
On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27044/
---
(Updated Oct. 22, 2014, 4:57 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Bugs: AURORA-830
https://issues.apache.org/jira/browse/AURORA-830
Repository: aurora
Description
---
This patch changes the scheduler such that the executor overhead can be
configured from the commandline.
Diffs
-
config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50
src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java
ed60447c798a97daceda4a3bba6ee9bcdcaedd0f
src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
048740953629a950c136179c9b880b8dce8fa932
src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
e9f251508257cd7287ff00773e0073a3cd130df8
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
283f7e1e22decfe1053bd5640e8283b40eeac592
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/state/TaskAssigner.java
4db9be86f2e7db08d12e0182914a7c5130301b13
src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
e96974764844b5d1a3a05f6996075fccee209594
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
c405d4c8b127c2dd7054c9520064da0346690f02
src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java
8ee84cda8670d117e2efa2d1a114da6f0d8315d6
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/27044/diff/
Testing
---
./gradlew clean build
Thanks,
Zameer Manji
Re: Review Request 27009: Use virtualenv to build pants instead of pex.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/#review57837 --- pants https://reviews.apache.org/r/27009/#comment98712 Why do we need to do this? - Zameer Manji On Oct. 22, 2014, 10:31 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/ --- (Updated Oct. 22, 2014, 10:31 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-876 https://issues.apache.org/jira/browse/AURORA-876 Repository: aurora Description --- Use virtualenv to build pants instead of pex. The goal of this change is to reduce CI flakiness by using `pip install` rather than `pex` to bootstrap pants. This still reaches out to external servers (and thus has the potential to be flaky) but is at least configurable (via `--default-timeout`). In a future review we can consider mirroring to svn.apache.org and disabling PyPI lookups. This requires a change to production executor code to use `sys.executable` rather than chmod+x-ing the runner PEX. Failure to do this results in the pants virtualenv's `site-packages` shadowing the dependencies in the `pex` (fairly obviously as there is a test failure related to psutil1/2 conflicts). I'm sure I'm papering over a PEX bug here but IMO this is fine as the only reason to execute the thermos runner PEX under a different python interpreter than the executor is using (based on the hash-bang in the pex) is not a configuration we need to support IMO. Diffs - pants 168419d7d8f5d89a0d87bfc6ac9f06c9e8e3dbc2 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 Diff: https://reviews.apache.org/r/27009/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27009: Use virtualenv to build pants instead of pex.
On Oct. 22, 2014, 10:59 a.m., Zameer Manji wrote: pants, line 40 https://reviews.apache.org/r/27009/diff/1/?file=728491#file728491line40 Why do we need to do this? We don't - removed. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/#review57837 --- On Oct. 22, 2014, 11:05 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/ --- (Updated Oct. 22, 2014, 11:05 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-876 https://issues.apache.org/jira/browse/AURORA-876 Repository: aurora Description --- Use virtualenv to build pants instead of pex. The goal of this change is to reduce CI flakiness by using `pip install` rather than `pex` to bootstrap pants. This still reaches out to external servers (and thus has the potential to be flaky) but is at least configurable (via `--default-timeout`). In a future review we can consider mirroring to svn.apache.org and disabling PyPI lookups. This requires a change to production executor code to use `sys.executable` rather than chmod+x-ing the runner PEX. Failure to do this results in the pants virtualenv's `site-packages` shadowing the dependencies in the `pex` (fairly obviously as there is a test failure related to psutil1/2 conflicts). I'm sure I'm papering over a PEX bug here but IMO this is fine as the only reason to execute the thermos runner PEX under a different python interpreter than the executor is using (based on the hash-bang in the pex) is not a configuration we need to support IMO. Diffs - pants 168419d7d8f5d89a0d87bfc6ac9f06c9e8e3dbc2 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 Diff: https://reviews.apache.org/r/27009/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27009: Use virtualenv to build pants instead of pex.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/ --- (Updated Oct. 22, 2014, 11:05 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Removed debugging statements. Bugs: AURORA-876 https://issues.apache.org/jira/browse/AURORA-876 Repository: aurora Description --- Use virtualenv to build pants instead of pex. The goal of this change is to reduce CI flakiness by using `pip install` rather than `pex` to bootstrap pants. This still reaches out to external servers (and thus has the potential to be flaky) but is at least configurable (via `--default-timeout`). In a future review we can consider mirroring to svn.apache.org and disabling PyPI lookups. This requires a change to production executor code to use `sys.executable` rather than chmod+x-ing the runner PEX. Failure to do this results in the pants virtualenv's `site-packages` shadowing the dependencies in the `pex` (fairly obviously as there is a test failure related to psutil1/2 conflicts). I'm sure I'm papering over a PEX bug here but IMO this is fine as the only reason to execute the thermos runner PEX under a different python interpreter than the executor is using (based on the hash-bang in the pex) is not a configuration we need to support IMO. Diffs (updated) - pants 168419d7d8f5d89a0d87bfc6ac9f06c9e8e3dbc2 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 Diff: https://reviews.apache.org/r/27009/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27047: Improve status command output ordering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27047/#review57841 --- Ship it! Ship It! - Zameer Manji On Oct. 22, 2014, 10:57 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27047/ --- (Updated Oct. 22, 2014, 10:57 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-879 https://issues.apache.org/jira/browse/aurora-879 Repository: aurora Description --- - Sort tasks by instance number. - Sort events by timestamp. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 10f8e0d331ca607e55e3aa6f96014caea744ed9f src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 Diff: https://reviews.apache.org/r/27047/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli:job Build operating on top level addresses: set([BuildFileAddress(/Users/mchucarroll/Code/incubator-aurora/src/test/python/apache/aurora/client/cli/BUILD, job)]) = test session starts == platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 58 items src/test/python/apache/aurora/client/cli/test_cancel_update.py .. src/test/python/apache/aurora/client/cli/test_create.py ... src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py src/test/python/apache/aurora/client/cli/test_open.py . src/test/python/apache/aurora/client/cli/test_restart.py src/test/python/apache/aurora/client/cli/test_status.py . == 58 passed in 3.65 seconds === src.test.python.apache.aurora.client.cli.job . SUCCESS Thanks, Mark Chu-Carroll
Re: Review Request 27009: Use virtualenv to build pants instead of pex.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/#review57843 --- Ship it! Please file a PEX bug about the behaviour you noticed as well. - Zameer Manji On Oct. 22, 2014, 11:05 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/ --- (Updated Oct. 22, 2014, 11:05 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-876 https://issues.apache.org/jira/browse/AURORA-876 Repository: aurora Description --- Use virtualenv to build pants instead of pex. The goal of this change is to reduce CI flakiness by using `pip install` rather than `pex` to bootstrap pants. This still reaches out to external servers (and thus has the potential to be flaky) but is at least configurable (via `--default-timeout`). In a future review we can consider mirroring to svn.apache.org and disabling PyPI lookups. This requires a change to production executor code to use `sys.executable` rather than chmod+x-ing the runner PEX. Failure to do this results in the pants virtualenv's `site-packages` shadowing the dependencies in the `pex` (fairly obviously as there is a test failure related to psutil1/2 conflicts). I'm sure I'm papering over a PEX bug here but IMO this is fine as the only reason to execute the thermos runner PEX under a different python interpreter than the executor is using (based on the hash-bang in the pex) is not a configuration we need to support IMO. Diffs - pants 168419d7d8f5d89a0d87bfc6ac9f06c9e8e3dbc2 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 Diff: https://reviews.apache.org/r/27009/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27047: Improve status command output ordering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27047/#review57851 --- src/test/python/apache/aurora/client/cli/test_status.py https://reviews.apache.org/r/27047/#comment98726 spec? - Joshua Cohen On Oct. 22, 2014, 5:57 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27047/ --- (Updated Oct. 22, 2014, 5:57 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-879 https://issues.apache.org/jira/browse/aurora-879 Repository: aurora Description --- - Sort tasks by instance number. - Sort events by timestamp. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 10f8e0d331ca607e55e3aa6f96014caea744ed9f src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 Diff: https://reviews.apache.org/r/27047/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli:job Build operating on top level addresses: set([BuildFileAddress(/Users/mchucarroll/Code/incubator-aurora/src/test/python/apache/aurora/client/cli/BUILD, job)]) = test session starts == platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 58 items src/test/python/apache/aurora/client/cli/test_cancel_update.py .. src/test/python/apache/aurora/client/cli/test_create.py ... src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py src/test/python/apache/aurora/client/cli/test_open.py . src/test/python/apache/aurora/client/cli/test_restart.py src/test/python/apache/aurora/client/cli/test_status.py . == 58 passed in 3.65 seconds === src.test.python.apache.aurora.client.cli.job . SUCCESS Thanks, Mark Chu-Carroll
Re: Review Request 27047: Improve status command output ordering.
On Oct. 22, 2014, 2:41 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_status.py, line 81 https://reviews.apache.org/r/27047/diff/1/?file=728832#file728832line81 spec? I'll do better than spec: there's no reason for that to be a mock at all. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27047/#review57851 --- On Oct. 22, 2014, 1:57 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27047/ --- (Updated Oct. 22, 2014, 1:57 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-879 https://issues.apache.org/jira/browse/aurora-879 Repository: aurora Description --- - Sort tasks by instance number. - Sort events by timestamp. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 10f8e0d331ca607e55e3aa6f96014caea744ed9f src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 Diff: https://reviews.apache.org/r/27047/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli:job Build operating on top level addresses: set([BuildFileAddress(/Users/mchucarroll/Code/incubator-aurora/src/test/python/apache/aurora/client/cli/BUILD, job)]) = test session starts == platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 58 items src/test/python/apache/aurora/client/cli/test_cancel_update.py .. src/test/python/apache/aurora/client/cli/test_create.py ... src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py src/test/python/apache/aurora/client/cli/test_open.py . src/test/python/apache/aurora/client/cli/test_restart.py src/test/python/apache/aurora/client/cli/test_status.py . == 58 passed in 3.65 seconds === src.test.python.apache.aurora.client.cli.job . SUCCESS Thanks, Mark Chu-Carroll
Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/ --- (Updated Oct. 22, 2014, 9:10 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Updating title and description. Summary (updated) - Preparing for Identity struct deprecation (client and executor). Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description (updated) --- Python side of changes for deprecating identity struct. Accounting for the upcoming deprecation by backfilling missing fields and switching between old/new fields (whichever is available). Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed together. Summary of changes: - TaskConfig - backfilling _key_ field in job diff and update commands. Also, some unit test refactoring to avoid copy-pasted fragments. Diffs - src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/api/updater.py 2e6bc9fdc1c745b2a69f8929209d46517c218700 src/main/python/apache/aurora/client/cli/jobs.py 0277cbed4b7eb927d6e5823b4b41d90b366f81d0 src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 src/test/python/apache/aurora/client/api/test_instance_watcher.py ae1b24bf4e3291cb31b3129cabcacdf32db0c560 src/test/python/apache/aurora/client/api/test_sla.py 1117f24d5ad3640632a1dd728913ba73c8bec707 src/test/python/apache/aurora/client/api/test_updater.py f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 95aa649cfff9166dd10aa432c4d470739e8f06c5 src/test/python/apache/aurora/client/cli/test_diff.py 10817695352687cdb5b0c3ed9720e3091b230e68 src/test/python/apache/aurora/client/cli/test_status.py c704daec5a6eee73c7092a201b168881853908e8 src/test/python/apache/aurora/client/cli/test_task_run.py 16fde14c03f6fd2c000e76625fad174835763f1b src/test/python/apache/aurora/client/cli/test_update.py cff1b6578aec6f5bcc1e610e58b47af233f32b41 src/test/python/apache/aurora/client/cli/util.py 3fa609a5f71525393ca0a5dbd81423005fadb583 src/test/python/apache/aurora/client/commands/test_diff.py c8d01456aa52fd61374b4f0960b5159da2cb235b src/test/python/apache/aurora/client/commands/test_ssh.py abb657ba397c23ddac6c6b188f70d1c4e34597a6 src/test/python/apache/aurora/client/commands/test_status.py 639763501348a35bff2f127e18780ac74852f51b src/test/python/apache/aurora/client/commands/test_update.py 07cbe53109e8bcdd09dcac47f6353b10e095717d src/test/python/apache/aurora/config/test_thrift.py 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 src/test/python/apache/aurora/executor/common/test_announcer.py 56943351ca09c29580dd764bb2442f0fcd9fde74 src/test/python/apache/aurora/executor/test_thermos_executor.py 65e8cce60a5543081175c574aaaf92f200bc6233 Diff: https://reviews.apache.org/r/26954/diff/ Testing --- ./pants src/test/python:all test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 27009: Use virtualenv to build pants instead of pex.
On Oct. 22, 2014, 2:06 p.m., Bill Farner wrote: pants, line 16 https://reviews.apache.org/r/27009/diff/2/?file=728833#file728833line16 Why allow unset? Virtualenv's activate script doesn't work under -o nounset. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/#review57896 --- On Oct. 22, 2014, 11:05 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/ --- (Updated Oct. 22, 2014, 11:05 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-876 https://issues.apache.org/jira/browse/AURORA-876 Repository: aurora Description --- Use virtualenv to build pants instead of pex. The goal of this change is to reduce CI flakiness by using `pip install` rather than `pex` to bootstrap pants. This still reaches out to external servers (and thus has the potential to be flaky) but is at least configurable (via `--default-timeout`). In a future review we can consider mirroring to svn.apache.org and disabling PyPI lookups. This requires a change to production executor code to use `sys.executable` rather than chmod+x-ing the runner PEX. Failure to do this results in the pants virtualenv's `site-packages` shadowing the dependencies in the `pex` (fairly obviously as there is a test failure related to psutil1/2 conflicts). I'm sure I'm papering over a PEX bug here but IMO this is fine as the only reason to execute the thermos runner PEX under a different python interpreter than the executor is using (based on the hash-bang in the pex) is not a configuration we need to support IMO. Diffs - pants 168419d7d8f5d89a0d87bfc6ac9f06c9e8e3dbc2 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 Diff: https://reviews.apache.org/r/27009/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27009: Use virtualenv to build pants instead of pex.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27009/ --- (Updated Oct. 22, 2014, 2:14 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Fix grammar in description. Bugs: AURORA-876 https://issues.apache.org/jira/browse/AURORA-876 Repository: aurora Description (updated) --- Use virtualenv to build pants instead of pex. The goal of this change is to reduce CI flakiness by using `pip install` rather than `pex` to bootstrap pants. This still reaches out to external servers (and thus has the potential to be flaky) but is at least configurable (via `--default-timeout`). In a future review we can consider mirroring to svn.apache.org and disabling PyPI lookups. This requires a change to production executor code to use `sys.executable` rather than chmod+x-ing the runner PEX. Failure to do this results in the pants virtualenv's `site-packages` shadowing the dependencies in the `pex` (fairly obviously as there is a test failure related to psutil1/2 conflicts). I'm sure I'm papering over a PEX bug here but IMO this is fine as the only reason for it is to execute the thermos runner PEX under a different python interpreter than the executor is using (based on the hash-bang in the pex). This is not a configuration we need to support IMO. Diffs - pants 168419d7d8f5d89a0d87bfc6ac9f06c9e8e3dbc2 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 Diff: https://reviews.apache.org/r/27009/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 26762: Preparing for Identity struct deprecation (scheduler).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26762/#review57400 --- Ship it! src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/26762/#comment98769 Is this safe to remove? I see red but no corresponding green. src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/26762/#comment98774 Does it make sense to reorder this for more code reuse and predictable outcome? - if key is not set, populate it - validate key _and_ owner src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java https://reviews.apache.org/r/26762/#comment98789 For other reviewers - this is safe because `SanitizedConfiguration.fromUnsanitized` requires consistency between `key` and `owner` fields, so worst case is that they are inconsistent (and would fail sanitization anyway) and you auth against the wrong user. src/main/thrift/org/apache/aurora/gen/api.thrift https://reviews.apache.org/r/26762/#comment98763 How about s/key/job/? It's less ambiguous (since we could conceivably introduce a TaskConfigKey), and sufficiently desriptive. - Bill Farner On Oct. 22, 2014, 9:06 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26762/ --- (Updated Oct. 22, 2014, 9:06 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- This patch replaces the Identity struct use within the scheduler and makes it ready for removal (i.e. when the JobStore is ready). Sending it as a separate CR for easier reviewing. Will have to be committed along with python changes (https://reviews.apache.org/r/26954/) to avoid breaking client diff functionality. Summary of the changes: * TaskConfig - dual write in StorageBackfill to populate new _key_ field. Incoming thrift objects are populated in ConfigurationManager during sanitizing. * TaskQuery - _owner_ to _role_ switch is handled in Query.Builder. All internal searching is now handled via _role_. * JobConfiguration - internal _owner_ refs redirected to _key.role_ 0.7.0. Diffs - src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 5c75cc8cae53edfa069c85c37ebad34774682081 src/main/java/org/apache/aurora/scheduler/TaskVars.java f1ab934541ad6d9ae74927f80a9c654a04922eb5 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/base/JobKeys.java a76c3fac71b35115064fba6644cff0066fd9e630 src/main/java/org/apache/aurora/scheduler/base/Query.java eded7a59eb394748b93d7fbc085a1bdf64b043cc src/main/java/org/apache/aurora/scheduler/base/Tasks.java 6ad79270c35c4fccb01f29d34ef1c4bbd7c953c8 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2 src/main/java/org/apache/aurora/scheduler/http/Utilization.java a0cb7bf56aeb7edd92b25d8d69a739d87452777a src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 5f08997f04ffa7d9610c2b41551943b563412626 src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 0f6731106c53420b92e60b9faf26c3614bd7ae00 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 58b94c2f2f3bac00f0692579974e8bdf159b6e40 src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 37176237fac336413267f3c8bb4e1b9a6255150c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 137f97d33decd14bf2f6dcdd9cd18c3db2b7c89c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 6ec130f4a9a5075b34452efb27c8fd0f08f93a63 src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 371ae87f5954fa5f092db1f6d21e2291d7576173 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 606c4434b7158220ccf1403b6deac939021fee31 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6
Re: Review Request 27045: Add some wiggle room before requiring min coverage thresholds be raised.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27045/#review57906 --- Ship it! Ship It! - Bill Farner On Oct. 22, 2014, 5:16 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27045/ --- (Updated Oct. 22, 2014, 5:16 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-880 https://issues.apache.org/jira/browse/AURORA-880 Repository: aurora Description --- Add some wiggle room before requiring min coverage thresholds be raised. Diffs - build.gradle 62d1492215313d6619491703fc88da3010f60886 buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 16f648b412b4d42c83c83da520358167b034ee25 Diff: https://reviews.apache.org/r/27045/diff/ Testing --- $ ./gradlew clean build [...] :jacocoTestReport Coverage report generated: file:///Users/jcohen/workspace/external/incubator-aurora/dist/reports/jacoco/test/html/index.html :analyzeReport Instruction coverage of 0.8806389838314789 exceeds minimum coverage of 0.87. Branch coverage of 0.825925925925926 exceeds minimum coverage of 0.82. :check :build BUILD SUCCESSFUL Thanks, Joshua Cohen
Re: Review Request 26531: Defining schema for the heartbeat RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26531/#review57911 --- Ship it! src/main/thrift/org/apache/aurora/gen/api.thrift https://reviews.apache.org/r/26531/#comment98806 -1 indent src/main/thrift/org/apache/aurora/gen/api.thrift https://reviews.apache.org/r/26531/#comment98816 Mind adding a note about what happens when an unknown job update is referenced? - Bill Farner On Oct. 21, 2014, 5:58 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26531/ --- (Updated Oct. 21, 2014, 5:58 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-690 https://issues.apache.org/jira/browse/AURORA-690 Repository: aurora Description --- Defining the flag and the schema for the update coordination (aka heartbeats). Diffs - build.gradle f4352d2ebf858930a6f219d96519e1cdc2bc14e5 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java e792d23d6bb13b4e61b078beea6d063f72f0d8fc src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 77032d84f91b149c01ce4ac62da7ca331a2b6445 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql db6a8719d18fee720f74ebcd2079ad36201cc831 src/main/thrift/org/apache/aurora/gen/api.thrift 401083621e1a1bbe43751b8bd168277543b9a812 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 3871dae68fcdc6402cb61a7244b46114617eecff src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 93f79d7cfed2ba3d56548b43b926ce7ddec16c9e src/test/python/apache/aurora/client/api/test_scheduler_client.py 076008e400bd9200cfd70fc469a4c310f291d70f Diff: https://reviews.apache.org/r/26531/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/#review57915 --- Is it worth the risk to update the client code now? Seems like we can ride it out until the field is removed with lower overall risk. - Bill Farner On Oct. 22, 2014, 9:10 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/ --- (Updated Oct. 22, 2014, 9:10 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Python side of changes for deprecating identity struct. Accounting for the upcoming deprecation by backfilling missing fields and switching between old/new fields (whichever is available). Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed together. Summary of changes: - TaskConfig - backfilling _key_ field in job diff and update commands. Also, some unit test refactoring to avoid copy-pasted fragments. Diffs - src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/api/updater.py 2e6bc9fdc1c745b2a69f8929209d46517c218700 src/main/python/apache/aurora/client/cli/jobs.py 0277cbed4b7eb927d6e5823b4b41d90b366f81d0 src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py f17910826c31c5bf754b43eb72500de639652f37 src/test/python/apache/aurora/client/api/test_instance_watcher.py ae1b24bf4e3291cb31b3129cabcacdf32db0c560 src/test/python/apache/aurora/client/api/test_sla.py 1117f24d5ad3640632a1dd728913ba73c8bec707 src/test/python/apache/aurora/client/api/test_updater.py f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 95aa649cfff9166dd10aa432c4d470739e8f06c5 src/test/python/apache/aurora/client/cli/test_diff.py 10817695352687cdb5b0c3ed9720e3091b230e68 src/test/python/apache/aurora/client/cli/test_status.py c704daec5a6eee73c7092a201b168881853908e8 src/test/python/apache/aurora/client/cli/test_task_run.py 16fde14c03f6fd2c000e76625fad174835763f1b src/test/python/apache/aurora/client/cli/test_update.py cff1b6578aec6f5bcc1e610e58b47af233f32b41 src/test/python/apache/aurora/client/cli/util.py 3fa609a5f71525393ca0a5dbd81423005fadb583 src/test/python/apache/aurora/client/commands/test_diff.py c8d01456aa52fd61374b4f0960b5159da2cb235b src/test/python/apache/aurora/client/commands/test_ssh.py abb657ba397c23ddac6c6b188f70d1c4e34597a6 src/test/python/apache/aurora/client/commands/test_status.py 639763501348a35bff2f127e18780ac74852f51b src/test/python/apache/aurora/client/commands/test_update.py 07cbe53109e8bcdd09dcac47f6353b10e095717d src/test/python/apache/aurora/config/test_thrift.py 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 src/test/python/apache/aurora/executor/common/test_announcer.py 56943351ca09c29580dd764bb2442f0fcd9fde74 src/test/python/apache/aurora/executor/test_thermos_executor.py 65e8cce60a5543081175c574aaaf92f200bc6233 Diff: https://reviews.apache.org/r/26954/diff/ Testing --- ./pants src/test/python:all test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57917 --- config/legacy_untested_classes.txt https://reviews.apache.org/r/27044/#comment98818 In the interest of this file being delete only, can you bite the bullet and create a unit test to cover these in a test? - Bill Farner On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 4:57 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 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/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 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/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Review Request 27057: Upgrade virtualenv to 1.11.6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27057/ --- Review request for Aurora, David McLaughlin and Joshua Cohen. Repository: aurora Description --- Upgrade virtualenv to 1.11.6. See changelog: https://virtualenv.pypa.io/en/latest/news.html Of note, this upgrades the bundled pip to one that supports https proxies. Diffs - build-support/virtualenv 00a56dfc05a98a756613dc39edefa48ded358314 Diff: https://reviews.apache.org/r/27057/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27057: Upgrade virtualenv to 1.11.6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27057/#review57920 --- Ship it! Ship It! - David McLaughlin On Oct. 22, 2014, 10:48 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27057/ --- (Updated Oct. 22, 2014, 10:48 p.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Repository: aurora Description --- Upgrade virtualenv to 1.11.6. See changelog: https://virtualenv.pypa.io/en/latest/news.html Of note, this upgrades the bundled pip to one that supports https proxies. Diffs - build-support/virtualenv 00a56dfc05a98a756613dc39edefa48ded358314 Diff: https://reviews.apache.org/r/27057/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57819 --- config/legacy_untested_classes.txt https://reviews.apache.org/r/27044/#comment98688 This seems to be out of order, revert? src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java https://reviews.apache.org/r/27044/#comment98819 Don't need a '\n' here. src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java https://reviews.apache.org/r/27044/#comment98820 This would be more compact: ``` .addResources(Resources.makeMesosResource( Resources.RAM_MB, slotFactory.getExecutorRAM().as(Data.MB))) ``` src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98821 Missing class javadoc. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98822 requireNonNull src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98823 Move this param to the next line. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98825 MorePreconditions.checkArgumentRange() src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98824 requireNonNull src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98826 Missing javadoc on non-getter public methods. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98827 Formatting is off. - Maxim Khutornenko On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 4:57 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 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/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 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/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27057: Upgrade virtualenv to 1.11.6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27057/#review57927 --- Looks like I jumped the gun a bit - this discovered a new pex issue. I've worked around it and filed https://github.com/pantsbuild/pex/issues/21 - Kevin Sweeney On Oct. 22, 2014, 3:48 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27057/ --- (Updated Oct. 22, 2014, 3:48 p.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Repository: aurora Description --- Upgrade virtualenv to 1.11.6. See changelog: https://virtualenv.pypa.io/en/latest/news.html Of note, this upgrades the bundled pip to one that supports https proxies. Diffs - build-support/virtualenv 00a56dfc05a98a756613dc39edefa48ded358314 Diff: https://reviews.apache.org/r/27057/diff/ Testing --- git clean -fdx ./pants src/test/python:all Thanks, Kevin Sweeney
Re: Review Request 27045: Add some wiggle room before requiring min coverage thresholds be raised.
On Oct. 22, 2014, 9:30 p.m., Bill Farner wrote: Ship It! Thanks! This is now on master: $ git log -1 origin/master --abbrev-commit commit cba7ec2 Author: Joshua Cohen jco...@twopensource.com Date: Wed Oct 22 16:11:29 2014 -0700 Add some wiggle room before requiring min coverage thresholds be raised. Bugs closed: AURORA-880 Reviewed at https://reviews.apache.org/r/27045/ - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27045/#review57906 --- On Oct. 22, 2014, 5:16 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27045/ --- (Updated Oct. 22, 2014, 5:16 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-880 https://issues.apache.org/jira/browse/AURORA-880 Repository: aurora Description --- Add some wiggle room before requiring min coverage thresholds be raised. Diffs - build.gradle 62d1492215313d6619491703fc88da3010f60886 buildSrc/src/main/groovy/org/apache/aurora/CoverageReportCheck.groovy 16f648b412b4d42c83c83da520358167b034ee25 Diff: https://reviews.apache.org/r/27045/diff/ Testing --- $ ./gradlew clean build [...] :jacocoTestReport Coverage report generated: file:///Users/jcohen/workspace/external/incubator-aurora/dist/reports/jacoco/test/html/index.html :analyzeReport Instruction coverage of 0.8806389838314789 exceeds minimum coverage of 0.87. Branch coverage of 0.825925925925926 exceeds minimum coverage of 0.82. :check :build BUILD SUCCESSFUL Thanks, Joshua Cohen
Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/#review57933 --- LG overall, aside for what seems to be unnecessary field modification when diffing configs. src/main/python/apache/aurora/client/api/updater.py https://reviews.apache.org/r/26954/#comment98842 Echoing our offline discussion - this is unnecessary, right? Since `from_config` and `to_config` are both from the scheduler, we can assume it is internally consistent? src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/26954/#comment98844 Ditto - should be unnecessary IIUC. src/test/python/apache/aurora/client/api/test_updater.py https://reviews.apache.org/r/26954/#comment98843 Ditto - this should be unnecessary, since both configs are round-tripped through the scheduler. src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/26954/#comment98845 While you're here, please change this to not use Mock, all down the hierarchy. Ditto for other cargo cults (thanks for collapsing a few, btw). - Bill Farner On Oct. 22, 2014, 11:11 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/ --- (Updated Oct. 22, 2014, 11:11 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Python side of changes for deprecating identity struct. Accounting for the upcoming deprecation by backfilling missing fields and switching between old/new fields (whichever is available). Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed together. Summary of changes: - TaskConfig - backfilling _key_ field in job diff and update commands. Also, some unit test refactoring to avoid copy-pasted fragments. Diffs - src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/api/updater.py 2e6bc9fdc1c745b2a69f8929209d46517c218700 src/main/python/apache/aurora/client/cli/jobs.py 10f8e0d331ca607e55e3aa6f96014caea744ed9f src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 src/test/python/apache/aurora/client/api/test_instance_watcher.py ae1b24bf4e3291cb31b3129cabcacdf32db0c560 src/test/python/apache/aurora/client/api/test_sla.py 1117f24d5ad3640632a1dd728913ba73c8bec707 src/test/python/apache/aurora/client/api/test_updater.py f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 95aa649cfff9166dd10aa432c4d470739e8f06c5 src/test/python/apache/aurora/client/cli/test_diff.py 10817695352687cdb5b0c3ed9720e3091b230e68 src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 src/test/python/apache/aurora/client/cli/test_task_run.py 16fde14c03f6fd2c000e76625fad174835763f1b src/test/python/apache/aurora/client/cli/test_update.py 1ec5483506a22a774340acccd33f09f1742be8b7 src/test/python/apache/aurora/client/cli/util.py 3fa609a5f71525393ca0a5dbd81423005fadb583 src/test/python/apache/aurora/client/commands/test_diff.py c8d01456aa52fd61374b4f0960b5159da2cb235b src/test/python/apache/aurora/client/commands/test_ssh.py abb657ba397c23ddac6c6b188f70d1c4e34597a6 src/test/python/apache/aurora/client/commands/test_status.py 639763501348a35bff2f127e18780ac74852f51b src/test/python/apache/aurora/client/commands/test_update.py 07cbe53109e8bcdd09dcac47f6353b10e095717d src/test/python/apache/aurora/config/test_thrift.py 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 src/test/python/apache/aurora/executor/common/test_announcer.py 56943351ca09c29580dd764bb2442f0fcd9fde74
Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/ --- (Updated Oct. 22, 2014, 11:59 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- CR comments. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Python side of changes for deprecating identity struct. Accounting for the upcoming deprecation by backfilling missing fields and switching between old/new fields (whichever is available). Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed together. Summary of changes: - TaskConfig - backfilling _key_ field in job diff and update commands. Also, some unit test refactoring to avoid copy-pasted fragments. Diffs (updated) - src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 src/test/python/apache/aurora/client/api/test_instance_watcher.py ae1b24bf4e3291cb31b3129cabcacdf32db0c560 src/test/python/apache/aurora/client/api/test_sla.py 1117f24d5ad3640632a1dd728913ba73c8bec707 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 95aa649cfff9166dd10aa432c4d470739e8f06c5 src/test/python/apache/aurora/client/cli/test_diff.py 10817695352687cdb5b0c3ed9720e3091b230e68 src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 src/test/python/apache/aurora/client/cli/test_task_run.py 16fde14c03f6fd2c000e76625fad174835763f1b src/test/python/apache/aurora/client/cli/test_update.py 1ec5483506a22a774340acccd33f09f1742be8b7 src/test/python/apache/aurora/client/cli/util.py 3fa609a5f71525393ca0a5dbd81423005fadb583 src/test/python/apache/aurora/client/commands/test_diff.py c8d01456aa52fd61374b4f0960b5159da2cb235b src/test/python/apache/aurora/client/commands/test_ssh.py abb657ba397c23ddac6c6b188f70d1c4e34597a6 src/test/python/apache/aurora/client/commands/test_status.py 639763501348a35bff2f127e18780ac74852f51b src/test/python/apache/aurora/client/commands/test_update.py 07cbe53109e8bcdd09dcac47f6353b10e095717d src/test/python/apache/aurora/config/test_thrift.py 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 src/test/python/apache/aurora/executor/common/test_announcer.py 56943351ca09c29580dd764bb2442f0fcd9fde74 src/test/python/apache/aurora/executor/test_thermos_executor.py 65e8cce60a5543081175c574aaaf92f200bc6233 Diff: https://reviews.apache.org/r/26954/diff/ Testing --- ./pants src/test/python:all test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/#review57939 --- In case you skimmed past it - the Mock thing would be great to address here. - Bill Farner On Oct. 22, 2014, 11:59 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/ --- (Updated Oct. 22, 2014, 11:59 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Python side of changes for deprecating identity struct. Accounting for the upcoming deprecation by backfilling missing fields and switching between old/new fields (whichever is available). Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed together. Summary of changes: - TaskConfig - backfilling _key_ field in job diff and update commands. Also, some unit test refactoring to avoid copy-pasted fragments. Diffs - src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 src/test/python/apache/aurora/client/api/test_instance_watcher.py ae1b24bf4e3291cb31b3129cabcacdf32db0c560 src/test/python/apache/aurora/client/api/test_sla.py 1117f24d5ad3640632a1dd728913ba73c8bec707 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 95aa649cfff9166dd10aa432c4d470739e8f06c5 src/test/python/apache/aurora/client/cli/test_diff.py 10817695352687cdb5b0c3ed9720e3091b230e68 src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 src/test/python/apache/aurora/client/cli/test_task_run.py 16fde14c03f6fd2c000e76625fad174835763f1b src/test/python/apache/aurora/client/cli/test_update.py 1ec5483506a22a774340acccd33f09f1742be8b7 src/test/python/apache/aurora/client/cli/util.py 3fa609a5f71525393ca0a5dbd81423005fadb583 src/test/python/apache/aurora/client/commands/test_diff.py c8d01456aa52fd61374b4f0960b5159da2cb235b src/test/python/apache/aurora/client/commands/test_ssh.py abb657ba397c23ddac6c6b188f70d1c4e34597a6 src/test/python/apache/aurora/client/commands/test_status.py 639763501348a35bff2f127e18780ac74852f51b src/test/python/apache/aurora/client/commands/test_update.py 07cbe53109e8bcdd09dcac47f6353b10e095717d src/test/python/apache/aurora/config/test_thrift.py 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 src/test/python/apache/aurora/executor/common/test_announcer.py 56943351ca09c29580dd764bb2442f0fcd9fde74 src/test/python/apache/aurora/executor/test_thermos_executor.py 65e8cce60a5543081175c574aaaf92f200bc6233 Diff: https://reviews.apache.org/r/26954/diff/ Testing --- ./pants src/test/python:all test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).
On Oct. 22, 2014, 11:51 p.m., Bill Farner wrote: src/test/python/apache/aurora/client/cli/test_diff.py, line 60 https://reviews.apache.org/r/26954/diff/3/?file=729386#file729386line60 While you're here, please change this to not use Mock, all down the hierarchy. Ditto for other cargo cults (thanks for collapsing a few, btw). Maxim Khutornenko wrote: I collapsed all related cli methods into util.py and using specs for all Mocks now. Did not modify the commands (v1) tests to let them die out naturally. I actually like mocks here as tests fail when accessing a field that is not populated. Please chime in on https://reviews.apache.org/r/27058/, where the exact opposite perspective exists. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/#review57933 --- On Oct. 22, 2014, 11:59 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26954/ --- (Updated Oct. 22, 2014, 11:59 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-84 https://issues.apache.org/jira/browse/AURORA-84 Repository: aurora Description --- Python side of changes for deprecating identity struct. Accounting for the upcoming deprecation by backfilling missing fields and switching between old/new fields (whichever is available). Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed together. Summary of changes: - TaskConfig - backfilling _key_ field in job diff and update commands. Also, some unit test refactoring to avoid copy-pasted fragments. Diffs - src/main/python/apache/aurora/client/api/instance_watcher.py b390aa8993205f1f6938f8c295e3c16a0bf4df6d src/main/python/apache/aurora/client/api/sla.py b9b64680b15f5395ed6aca681b9b1c30ffe2822c src/main/python/apache/aurora/client/cli/task.py c41484bdc27266443bc4e139e1ebb362a59be0f9 src/main/python/apache/aurora/client/commands/admin.py deee0250f3ba9837feeb92acc654f5b3b68b4e0f src/main/python/apache/aurora/client/commands/core.py 58f419e674f1a9a0ae9da6faa2e39c8167bab597 src/main/python/apache/aurora/client/commands/ssh.py d2b8bf675556b924d3d63b545d036dc48a081486 src/main/python/apache/aurora/config/thrift.py 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f src/main/python/apache/aurora/executor/aurora_executor.py 2c6423d096656f426a4385f4edef6875ebad7049 src/main/python/apache/aurora/executor/common/announcer.py 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 src/main/python/apache/aurora/executor/thermos_task_runner.py bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 src/test/python/apache/aurora/client/api/test_instance_watcher.py ae1b24bf4e3291cb31b3129cabcacdf32db0c560 src/test/python/apache/aurora/client/api/test_sla.py 1117f24d5ad3640632a1dd728913ba73c8bec707 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 95aa649cfff9166dd10aa432c4d470739e8f06c5 src/test/python/apache/aurora/client/cli/test_diff.py 10817695352687cdb5b0c3ed9720e3091b230e68 src/test/python/apache/aurora/client/cli/test_status.py 4f62cf0c52e5837309cf7ad702df6d907df8f510 src/test/python/apache/aurora/client/cli/test_task_run.py 16fde14c03f6fd2c000e76625fad174835763f1b src/test/python/apache/aurora/client/cli/test_update.py 1ec5483506a22a774340acccd33f09f1742be8b7 src/test/python/apache/aurora/client/cli/util.py 3fa609a5f71525393ca0a5dbd81423005fadb583 src/test/python/apache/aurora/client/commands/test_diff.py c8d01456aa52fd61374b4f0960b5159da2cb235b src/test/python/apache/aurora/client/commands/test_ssh.py abb657ba397c23ddac6c6b188f70d1c4e34597a6 src/test/python/apache/aurora/client/commands/test_status.py 639763501348a35bff2f127e18780ac74852f51b src/test/python/apache/aurora/client/commands/test_update.py 07cbe53109e8bcdd09dcac47f6353b10e095717d src/test/python/apache/aurora/config/test_thrift.py 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 src/test/python/apache/aurora/executor/common/test_announcer.py 56943351ca09c29580dd764bb2442f0fcd9fde74 src/test/python/apache/aurora/executor/test_thermos_executor.py 65e8cce60a5543081175c574aaaf92f200bc6233 Diff: https://reviews.apache.org/r/26954/diff/ Testing --- ./pants src/test/python:all test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/#review57936
---
src/test/python/apache/aurora/client/cli/test_diff.py
https://reviews.apache.org/r/27058/#comment98846
Feel free to drop this TODO as it is fixed in
https://reviews.apache.org/r/26954.
The caveat: setup_populate_job_config(). It uses the result of
create_mock_scheduled_tasks() (list of ScheduledTask) to populate a set of
TaskConfigs.
src/test/python/apache/aurora/client/commands/test_kill.py
https://reviews.apache.org/r/27058/#comment98850
4 spaces
- Maxim Khutornenko
On Oct. 22, 2014, 11:18 p.m., David McLaughlin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/
---
(Updated Oct. 22, 2014, 11:18 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 =
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
On Oct. 22, 2014, 11:24 p.m., Mark Chu-Carroll wrote:
src/test/python/apache/aurora/client/cli/test_command_hooks.py, line 252
https://reviews.apache.org/r/27058/diff/1/?file=729290#file729290line252
I don't think this needs to be a mock at all - I'm pretty sure that you
can just populate a real Response object directly. It looks like a lot of
the others are like this.
Kevin Sweeney wrote:
+1, and since the thrift structs work off dynamic properties, spec is
useless here. Calling the generated kwargs constructor gives you better
coverage here, as you can't accidentally create a thrift struct with a field
that doesn't exist without a TypeError
That's not true. Spec works just fine with thrift objects. For example, adding
'spec=TaskConfig' generates an error where the same test would previously pass:
AttributeError: Mock object has no attribute 'executorConfig'
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/#review57926
---
On Oct. 22, 2014, 11:18 p.m., David McLaughlin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/
---
(Updated Oct. 22, 2014, 11:18 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()
Review Request 27062: Fix typo in pants bash script.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27062/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- Forgot to update the review at https://reviews.apache.org/r/27062/ to match my local branch. Diffs - pants 1decdcb1dfa6b50b1d905fddd8c7da2ff90c4ec3 Diff: https://reviews.apache.org/r/27062/diff/ Testing --- % rm -fr build-support/pants.venv % ./pants % ls -l build-support/pants.venv total 16 drwxrwxr-x 2 ksweeney ksweeney 4096 Oct 22 17:37 bin -rw-rw-r-- 1 ksweeney ksweeney7 Oct 22 17:37 BOOTSTRAPPED drwxrwxr-x 2 ksweeney ksweeney 4096 Oct 22 17:36 include drwxrwxr-x 3 ksweeney ksweeney 4096 Oct 22 17:36 lib Thanks, Kevin Sweeney
Re: Review Request 27062: Fix typo in pants bash script.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27062/#review57948 --- Ship it! Ship It! - Bill Farner On Oct. 23, 2014, 12:38 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27062/ --- (Updated Oct. 23, 2014, 12:38 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- Forgot to update the review at https://reviews.apache.org/r/27062/ to match my local branch. Diffs - pants 1decdcb1dfa6b50b1d905fddd8c7da2ff90c4ec3 Diff: https://reviews.apache.org/r/27062/diff/ Testing --- % rm -fr build-support/pants.venv % ./pants % ls -l build-support/pants.venv total 16 drwxrwxr-x 2 ksweeney ksweeney 4096 Oct 22 17:37 bin -rw-rw-r-- 1 ksweeney ksweeney7 Oct 22 17:37 BOOTSTRAPPED drwxrwxr-x 2 ksweeney ksweeney 4096 Oct 22 17:36 include drwxrwxr-x 3 ksweeney ksweeney 4096 Oct 22 17:36 lib Thanks, Kevin Sweeney
Re: Review Request 27063: Freeze pants requirements.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27063/#review57956 --- Ship it! Should we file a bug against pants to pin their dependencies? pants https://reviews.apache.org/r/27063/#comment98868 Did you create the requirements file manually, or is there some easier/better way to update this in the future as pants changes its dependencies? - Joshua Cohen On Oct. 23, 2014, 1:38 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27063/ --- (Updated Oct. 23, 2014, 1:38 a.m.) Review request for Aurora, Joshua Cohen, Joe Smith, and Bill Farner. Repository: aurora Description --- This will help us avoid flaky builds in the future (some of pants's transitive dependencies use unqualified package names). Diffs - build-support/pants_requirements.txt PRE-CREATION pants 92585255774ef4a476a17c1102eb3e69c588cc88 Diff: https://reviews.apache.org/r/27063/diff/ Testing --- rm -fr build-support/pants.venv ./pants src/test/python:all Thanks, Kevin Sweeney
Review Request 27066: Follow the pantsbuild pants_support_baseurls for OS X Yosemite support.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27066/
---
Review request for Aurora, Kevin Sweeney and Bill Farner.
Bugs: AURORA-867
https://issues.apache.org/jira/browse/AURORA-867
Repository: aurora
Description
---
Follow the pantsbuild pants_support_baseurls for OS X Yosemite support.
Diffs
-
pants.ini 03f7af4fdb7f46c716da2c7e1da98ae5607e5d3c
Diff: https://reviews.apache.org/r/27066/diff/
Testing
---
22:19:24 incubator-aurora $ ./pants ./src/test/python/apache/aurora/admin:all
snip
BinaryNotFound: Failed to fetch binary ('bin/thrift', '0.5.0-finagle',
u'thrift') from any source: (Failed to fetch binary from
http://maven.twttr.com/twitter-commons/pants/build-support/bin/thrift/mac/10.10/0.5.0-finagle/thrift:
HTTP Error 404: Not Found)
After change:
22:40:44 incubator-aurora $ ./pants ./src/test/python/apache/aurora/admin:all
Build operating on top level addresses:
set([BuildFileAddress(/Users/joe/Development/incubator-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 2.13 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 1.40 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 27058: Add specs to instances of Mock in Python tests.
On Oct. 22, 2014, 4:24 p.m., Mark Chu-Carroll wrote:
src/test/python/apache/aurora/client/cli/test_command_hooks.py, line 252
https://reviews.apache.org/r/27058/diff/1/?file=729290#file729290line252
I don't think this needs to be a mock at all - I'm pretty sure that you
can just populate a real Response object directly. It looks like a lot of
the others are like this.
Kevin Sweeney wrote:
+1, and since the thrift structs work off dynamic properties, spec is
useless here. Calling the generated kwargs constructor gives you better
coverage here, as you can't accidentally create a thrift struct with a field
that doesn't exist without a TypeError
Maxim Khutornenko wrote:
That's not true. Spec works just fine with thrift objects. For example,
adding 'spec=TaskConfig' generates an error where the same test would
previously pass:
AttributeError: Mock object has no attribute 'executorConfig'
David McLaughlin wrote:
This is the behavior I observed as well. For example, see where I had to
update failure_count to failureCount because I added the spec.
I'd really prefer a separate ticket for swapping out mocks for real
thrift objects.
+1 on specs for thrift structs.
@dmcg: I'm good with a separate JIRA if you cut it and add the 'newbie' label.
- Joe
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/#review57926
---
On Oct. 22, 2014, 4:18 p.m., David McLaughlin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/
---
(Updated Oct. 22, 2014, 4:18 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()
