Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
On Jan. 26, 2015, 5:29 p.m., Dave Lester wrote: Were you able to test this patch on GitHub? I just tried applying this to my own GitHub account's Aurora repo (https://github.com/davelester/incubator-aurora) and used a second GH account to simulate this interaction, but never saw the prompt. Do you know if symlinks are supported here? Alternatively, we may want to move our existing contributions file to the root directory. Bill Farner wrote: I'd also be okay with moving the file. Jeffrey Schroeder wrote: would you like me to update this review to just git mv the file or do you want to do it yourself? Please update the review. This sounds lazy, but it's helpful for our stats if the commit goes in as you :-) - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/#review69625 --- On Jan. 24, 2015, 9:53 p.m., Jeffrey Schroeder wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 24, 2015, 9:53 p.m.) Review request for Aurora. Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs - CONTRIBUTING.md PRE-CREATION Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 26, 2015, 6:53 p.m.) Review request for Aurora and Bill Farner. Changes --- People += wfarner -wfarner Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs - CONTRIBUTING.md PRE-CREATION Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Re: Review Request 30204: Upgrade to rbt=0.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30204/#review69653 --- Ship it! Ship It! - Kevin Sweeney On Jan. 22, 2015, 6:11 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30204/ --- (Updated Jan. 22, 2015, 6:11 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Release notes: https://www.reviewboard.org/docs/releasenotes/rbtools/0.7/ Some potentially-interesting new features: ``` rbt land The new rbt land command is a quick and easy way to push a change that has been reviewed on Review Board to the upstream repository. The change may be in a local branch or stored as a patch on Review Board. When running against Review Board 2.0+, this command will start by checking the approval state of the review request (on older versions, it just checks for one or more “Ship It!”s). If the change is approved, the patch will be applied just like rbt patch. The change can optionally be pushed to the remote repository using the -p/--push command line option. This is currently only available when using Git repositories. rbt stamp The new rbt stamp command will amend a commit message with a “Reviewed at url” line. This feature is especially useful when using the close-on-submit hooks in Review Board 2.x to close out review requests when the changes are committed. This is currently only supported for Git repositories. Patch by Yanjia (Nicole) Xin. ``` Diffs - rbt 0742cf47924ee013758883878fa229b5b876be27 Diff: https://reviews.apache.org/r/30204/diff/ Testing --- Posted this review. Thanks, Bill Farner
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69663 --- src/main/python/apache/aurora/common/clusters.py https://reviews.apache.org/r/30187/#comment114397 For the purposes of sheparding this review along would you consider moving this to another change? - Kevin Sweeney On Jan. 22, 2015, 1:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 1:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
On Jan. 26, 2015, 8:16 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/common/clusters.py, line 42 https://reviews.apache.org/r/30187/diff/1/?file=830286#file830286line42 For the purposes of sheparding this review along would you consider moving this to another change? Can you give more detail on the reasoning? While i generally agree with keeping logically-different changes separate, i don't think we should be strictly opposed to cleaning up code in the immediate vicinity of a patch. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69663 --- On Jan. 22, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 9:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30203: Fix impedance mismatch between offer matching and task launching.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30203/ --- (Updated Jan. 26, 2015, 8:18 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Changes --- Rebased + fixed merge conflicts. Bugs: AURORA-1050 https://issues.apache.org/jira/browse/AURORA-1050 Repository: aurora Description --- See linked ticket for context on how this manifested. Please don't be overwhelmed by the large delta in this diff - a majority of it is reorganizing code to live in more appropriate places. This happened because the logic for offer matching and task launching were out of sync. For small tasks (smaller than `MIN_THERMOS_RESOURCES`), an additional amount (`MIN_TASK_RESOURCES`) would be unintentionally added when the task was launched, but this was not considered when comparing the task to offers. The test case `MesosTaskFactoryImplTest.testSmallTaskUpsizing` was added to reproduce this bug. This change does several things to make the situation more sane: - `ResourceSlot` no longer directly accesses command line arguments, to simplify testing - You may no longer create a `ResourceSlot` from its constitutent parts, to prevent accidental misuse - The algorithm used in `ResourceSlot` was simplified such that an epsilon is always used for the executor resources, and that is subtracted from the final resources required by the task. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 0b15834ec67959d3be94f9a5240ed38f43ac4c5b src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java dd64a2250ab40663fc7a0296c11206aaed178d7f src/main/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterState.java e6bd1b517535cafce4976e585b377065dfd19796 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 024a689d788804e95de76570674b6d4aa77d7495 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 0204d14b19ae412236f19ca274d81decb4eba12d src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 65c4b526c89a4d5607af4424ebe49bb48e296ae9 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java c2a342ce07bfb223193886038761f0da5230135d src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 1cb56f19c331508a1585077e9c4a98f52aac343b src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 846914eeba38004e5e7d61ccb76b5219442d8daf src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 7ba946422577c21cbc3b3edf8d30ee313b0ef251 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 65b4836d39f70295264a58e3227863fa4ced705c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 36dbcf73686c5a3ade01f7a10fda8ac4bdbcc7ad src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 0b41156f2a574d3d3c2cf840926f307dfb1e726e src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 265c38d20136210e7639ac8ea915d307a4b72949 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 22faccf42c3cc1181ef2830cd0b86a3146779d14 src/test/java/org/apache/aurora/scheduler/mesos/Offers.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java PRE-CREATION src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 67d3dbb6ea02baacfbed72e9d70c3109b3a37759 src/test/sh/org/apache/aurora/e2e/validate_serverset.py 66fa965044a4998857b6458582eeb3296558168d Diff: https://reviews.apache.org/r/30203/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30203: Fix impedance mismatch between offer matching and task launching.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30203/#review69666 --- Master (7ba6226) is red with this patch. ./build-support/jenkins/build.sh :assemble :compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7' 1 warning :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava :processTestResources :testClasses :checkstyleTest :findbugsJmh :findbugsMain :findbugsTest :licenseJmh UP-TO-DATE :licenseMain UP-TO-DATE :licenseTest UP-TO-DATE :license UP-TO-DATE :pmdMain :test :jacocoTestReport Coverage report generated: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html :analyzeReport Instruction coverage of 0.8913210893561411 exceeds minimum coverage of 0.89. :analyzeReport FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':analyzeReport'. Branch coverage is 0.8343558282208589, but must be greater than 0.835 * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 3 mins 46.842 secs I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Jan. 26, 2015, 8:18 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30203/ --- (Updated Jan. 26, 2015, 8:18 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1050 https://issues.apache.org/jira/browse/AURORA-1050 Repository: aurora Description --- See linked ticket for context on how this manifested. Please don't be overwhelmed by the large delta in this diff - a majority of it is reorganizing code to live in more appropriate places. This happened because the logic for offer matching and task launching were out of sync. For small tasks (smaller than `MIN_THERMOS_RESOURCES`), an additional amount (`MIN_TASK_RESOURCES`) would be unintentionally added when the task was launched, but this was not considered when comparing the task to offers. The test case `MesosTaskFactoryImplTest.testSmallTaskUpsizing` was added to reproduce this bug. This change does several things to make the situation more sane: - `ResourceSlot` no longer directly accesses command line arguments, to simplify testing - You may no longer create a `ResourceSlot` from its constitutent parts, to prevent accidental misuse - The algorithm used in `ResourceSlot` was simplified such that an epsilon is always used for the executor resources, and that is subtracted from the final resources required by the task. Diffs - src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 0b15834ec67959d3be94f9a5240ed38f43ac4c5b src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java dd64a2250ab40663fc7a0296c11206aaed178d7f src/main/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterState.java e6bd1b517535cafce4976e585b377065dfd19796 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 024a689d788804e95de76570674b6d4aa77d7495 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 0204d14b19ae412236f19ca274d81decb4eba12d src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 65c4b526c89a4d5607af4424ebe49bb48e296ae9 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java c2a342ce07bfb223193886038761f0da5230135d src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 1cb56f19c331508a1585077e9c4a98f52aac343b src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 846914eeba38004e5e7d61ccb76b5219442d8daf src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 7ba946422577c21cbc3b3edf8d30ee313b0ef251 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 65b4836d39f70295264a58e3227863fa4ced705c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 36dbcf73686c5a3ade01f7a10fda8ac4bdbcc7ad src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 0b41156f2a574d3d3c2cf840926f307dfb1e726e src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 265c38d20136210e7639ac8ea915d307a4b72949
Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
On Jan. 26, 2015, 5:29 p.m., Dave Lester wrote: Were you able to test this patch on GitHub? I just tried applying this to my own GitHub account's Aurora repo (https://github.com/davelester/incubator-aurora) and used a second GH account to simulate this interaction, but never saw the prompt. Do you know if symlinks are supported here? Alternatively, we may want to move our existing contributions file to the root directory. I'd also be okay with moving the file. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/#review69625 --- On Jan. 24, 2015, 9:53 p.m., Jeffrey Schroeder wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 24, 2015, 9:53 p.m.) Review request for Aurora. Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs - CONTRIBUTING.md PRE-CREATION Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Re: Review Request 30248: Fix a markdown syntax error in the deployment docs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30248/#review69620 --- docs/deploying-aurora-scheduler.md https://reviews.apache.org/r/30248/#comment114340 Nice catch. Could you also correct the link here? It should `monitoring.md` vs `docs/monitoring.md` - Dave Lester On Jan. 24, 2015, 9:48 p.m., Jeffrey Schroeder wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30248/ --- (Updated Jan. 24, 2015, 9:48 p.m.) Review request for Aurora. Repository: aurora Description --- Fix a markdown syntax error in the deployment docs Diffs - docs/deploying-aurora-scheduler.md c14f865a844b6eb34cc0e9363dba50d4975f5632 Diff: https://reviews.apache.org/r/30248/diff/ Testing --- Rendered document before and then after. Thanks, Jeffrey Schroeder
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
On Jan. 26, 2015, 10:14 a.m., Aurora ReviewBot wrote: Master (7ba6226) is red with this patch. ./build-support/jenkins/build.sh :assemble :compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7' 1 warning :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava :processTestResources :testClasses :checkstyleTest :findbugsJmh :findbugsMain :findbugsTest :licenseJmh UP-TO-DATE :licenseMain UP-TO-DATE :licenseTest UP-TO-DATE :license UP-TO-DATE :pmdMain :test :jacocoTestReport Coverage report generated: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html :analyzeReport Instruction coverage of 0.8917038316949588 exceeds minimum coverage of 0.89. :analyzeReport FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':analyzeReport'. Branch coverage is 0.8343558282208589, but must be greater than 0.835 * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 3 mins 23.668 secs I will refresh this build result if you post a review containing @ReviewBot retry Branch coverage is 0.8343558282208589, but must be greater than 0.835 Please ignore this failure, as you obviously did not affect branch coverage. Looking into this at https://issues.apache.org/jira/browse/AURORA-1060. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/#review69594 --- On Jan. 25, 2015, 8:10 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 25, 2015, 8:10 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 5dfbcf1f6de716502a28f7da33a095968eb8420e src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 92ba45033ada8114349c435316c9681395aea706 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
On Jan. 26, 2015, 5:29 p.m., Dave Lester wrote: Were you able to test this patch on GitHub? I just tried applying this to my own GitHub account's Aurora repo (https://github.com/davelester/incubator-aurora) and used a second GH account to simulate this interaction, but never saw the prompt. Do you know if symlinks are supported here? Alternatively, we may want to move our existing contributions file to the root directory. Bill Farner wrote: I'd also be okay with moving the file. would you like me to update this review to just git mv the file or do you want to do it yourself? - Jeffrey --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/#review69625 --- On Jan. 24, 2015, 9:53 p.m., Jeffrey Schroeder wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 24, 2015, 9:53 p.m.) Review request for Aurora. Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs - CONTRIBUTING.md PRE-CREATION Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69668 --- src/main/python/apache/aurora/common/clusters.py https://reviews.apache.org/r/30187/#comment114399 wait -- the # noqa is necessary, otherwise checkstyle will fail. - Brian Wickman On Jan. 22, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 9:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
On Jan. 26, 2015, 12:16 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/common/clusters.py, line 42 https://reviews.apache.org/r/30187/diff/1/?file=830286#file830286line42 For the purposes of sheparding this review along would you consider moving this to another change? Bill Farner wrote: Can you give more detail on the reasoning? While i generally agree with keeping logically-different changes separate, i don't think we should be strictly opposed to cleaning up code in the immediate vicinity of a patch. The patch that removes the style-checker annotations around this set of files will need to be accompanied by a change to the checkstyle tool to ignore or detect these violations. IMO that's big enough for a separate patch. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69663 --- On Jan. 22, 2015, 1:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 1:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30286: Raise .auroraversion to 0.7.0-SNAPSHOT
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30286/#review69686 --- Ship it! Ship It! - Bill Farner On Jan. 26, 2015, 9:27 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30286/ --- (Updated Jan. 26, 2015, 9:27 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Bumping up aurora snapshot version in preparation for the 0.7.0 release. Diffs - src/main/resources/apache/aurora/client/cli/.auroraversion e8ff9d45c6326c28dae836a1409ec0c9b00fd75a Diff: https://reviews.apache.org/r/30286/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 30285: Add Protobufs anonymous class back to untested classes list.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30285/#review69682 --- Ship it! Ship It! - David McLaughlin On Jan. 26, 2015, 9:05 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30285/ --- (Updated Jan. 26, 2015, 9:05 p.m.) Review request for Aurora and Zameer Manji. Repository: aurora Description --- This broke as a result of me reverting a change done in https://reviews.apache.org/r/28920/ when resolving merge conflicts. Prior to that patch, a call to `LOG.fine` was guarded by an if statement, which was removed. I reverted that since the purpose of the guard was performance-related (protobuf's `toString()` function can be very expensive). For some unknown reason my local builds continued to pass after this change, despite failing in jenkins. This was made worse by https://issues.apache.org/jira/browse/AURORA-1060, which caused review bot to fail at a point before this check was done. Diffs - config/legacy_untested_classes.txt 3429e35376cf71863dcc63c9c667aa70b0ec6b22 Diff: https://reviews.apache.org/r/30285/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30207: Simplify AuroraCommandContext
On Jan. 23, 2015, 8:47 p.m., Maxim Khutornenko wrote: I am not convinced there is enough value in this diff to risk possible regression. Besides, the majority of what this diff touches will die out along with the client updater. Zameer Manji wrote: Is there an ETA for the destruction of the client updater? Maxim Khutornenko wrote: Any time we feel ready to drop beta from scheduler updater. Zameer Manji wrote: I'm willing to drop this diff, if you are willing to start the conversation on when we can drop 'beta' from the scheduler updater. Maxim Khutornenko wrote: How is that related? :) There is nothing pressing us to graduate scheduler updater at this point. There are still bugs to fix and parity features to implement (e.g. heartbeats) before we are ready for prime time. Bill Farner wrote: I share Maxim's general uneasiness about changing behavior in the client-side updater since it is complex and sunsetting. However, i don't share the concern in this diff. The change appears to be very straightforward, especially in `update.py`. Maxim - is there any particular part you're worried about? Maxim Khutornenko wrote: I just don't see a reason to shuffle things around (no matter how trivial it looks) for a feature that is going away. I view the value of refactoring as making a long term positive impact on readabilty and reusability. This change does not clear the bar for me. The important context is that this is not a refactor in service of the updater, but to unwind context.py, which has very high fan-in within the client. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30207/#review69458 --- On Jan. 23, 2015, 3:32 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30207/ --- (Updated Jan. 23, 2015, 3:32 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- The AuroraCommandContext class is used in multiple commands and contains common code for all of them. However some portions are only used by one command. This patch takes some of those portions and moves them to the command that requires that functionality. Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/update.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_update.py 8b7d11202b35deb09a248cfe0a96458fb70c Diff: https://reviews.apache.org/r/30207/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Zameer Manji
Review Request 30285: Add Protobufs anonymous class back to untested classes list.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30285/ --- Review request for Aurora and Zameer Manji. Repository: aurora Description --- This broke as a result of me reverting a change done in https://reviews.apache.org/r/28920/ when resolving merge conflicts. Prior to that patch, a call to `LOG.fine` was guarded by an if statement, which was removed. I reverted that since the purpose of the guard was performance-related (protobuf's toString() function can be very expensive. For some unknown reason my local builds continued to pass after this change, despite failing in jenkins. This was made worse by https://issues.apache.org/jira/browse/AURORA-1060, which caused review bot to fail at a point before this check was done. Diffs - config/legacy_untested_classes.txt 3429e35376cf71863dcc63c9c667aa70b0ec6b22 Diff: https://reviews.apache.org/r/30285/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 27, 2015, 4:27 a.m.) Review request for Aurora and Bill Farner. Changes --- Updated per review comments. Moved docs/contributing.md -- CONTRIBUTING.md per the github documentation. Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs (updated) - docs/README.md 4889b3ff208d56e3af34fdd876efaa7719e6504f docs/contributing.md Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
On Jan. 26, 2015, 5:29 p.m., Dave Lester wrote: Were you able to test this patch on GitHub? I just tried applying this to my own GitHub account's Aurora repo (https://github.com/davelester/incubator-aurora) and used a second GH account to simulate this interaction, but never saw the prompt. Do you know if symlinks are supported here? Alternatively, we may want to move our existing contributions file to the root directory. Bill Farner wrote: I'd also be okay with moving the file. Jeffrey Schroeder wrote: would you like me to update this review to just git mv the file or do you want to do it yourself? Bill Farner wrote: Please update the review. This sounds lazy, but it's helpful for our stats if the commit goes in as you :-) No I'd much prefer to get the review attributed to me, that is part of the draw of using git instead of svn (like zookeeper meh...). It is easier to do drive by fixes like this when I notice them and still get the credit. - Jeffrey --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/#review69625 --- On Jan. 27, 2015, 4:27 a.m., Jeffrey Schroeder wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 27, 2015, 4:27 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs - docs/README.md 4889b3ff208d56e3af34fdd876efaa7719e6504f docs/contributing.md Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Review Request 30302: Updating release publishing script to work with linked .auroraversion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30302/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- The release publishing script needs a fix for git add .auroraversion. Diffs - build-support/release/release 147568b86586940648c9511e5eff7d4b56fe18a1 Diff: https://reviews.apache.org/r/30302/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
On Jan. 26, 2015, 11:19 nachm., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java, line 413 https://reviews.apache.org/r/30010/diff/6/?file=833613#file833613line413 s/public // Feeling kind of stupid here, I somehow thought this had to be public so that I could use it in the ConfigurationManagerTest. - Florian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/#review69688 --- On Jan. 27, 2015, 1:08 vorm., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 27, 2015, 1:08 vorm.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 5dfbcf1f6de716502a28f7da33a095968eb8420e src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 92ba45033ada8114349c435316c9681395aea706 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30010/#review69724
---
Master (0cb40d1) is red with this patch.
./build-support/jenkins/build.sh
File build/bdist.linux-x86_64/egg/pkg_resources.py, line 830, in obtain
for item in search_path:
File build/bdist.linux-x86_64/egg/setuptools/dist.py, line 314, in
fetch_build_egg
File build/bdist.linux-x86_64/egg/setuptools/command/easy_install.py, line
581, in easy_install
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 591, in
fetch_distribution
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 428, in
find_packages
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 769, in
scan_url
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 326, in
process_url
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 405, in
process_index
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 769, in
scan_url
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 305, in
process_url
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 703, in
open_url
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 902, in
_socket_timeout
File build/bdist.linux-x86_64/egg/setuptools/package_index.py, line 1015,
in open_with_auth
File /usr/lib/python2.7/urllib2.py, line 404, in open
response = self._open(req, data)
File /usr/lib/python2.7/urllib2.py, line 422, in _open
'_open', req)
File /usr/lib/python2.7/urllib2.py, line 382, in _call_chain
result = func(*args)
File /usr/lib/python2.7/urllib2.py, line 1214, in http_open
return self.do_open(httplib.HTTPConnection, req)
File /usr/lib/python2.7/urllib2.py, line 1187, in do_open
r = h.getresponse(buffering=True)
File /usr/lib/python2.7/httplib.py, line 1045, in getresponse
response.begin()
File /usr/lib/python2.7/httplib.py, line 409, in begin
version, status, reason = self._read_status()
File /usr/lib/python2.7/httplib.py, line 365, in _read_status
line = self.fp.readline(_MAXLINE + 1)
File /usr/lib/python2.7/socket.py, line 476, in readline
data = self._sock.recv(self._rbufsize)
socket.timeout: timed out
[31m
FAILURE[0m
Exception message: Package
SourcePackage('file:///home/jenkins/jenkins-slave/workspace/AuroraBot/.pants.d/python/eggs/protobuf-3.0.0-alpha-1.tar.gz')
is not translateable.
I will refresh this build result if you post a review containing @ReviewBot
retry
- Aurora ReviewBot
On Jan. 27, 2015, 1:08 a.m., Florian Pfeiffer wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30010/
---
(Updated Jan. 27, 2015, 1:08 a.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Bugs: AURORA-184
https://issues.apache.org/jira/browse/AURORA-184
Repository: aurora
Description
---
[AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
This is the first step for AURORA-184, that removes the default hostrack
limit constraints.
The second step that's still missing would be to add s.th. like
--default-constraints as start parameter to the scheduler.
AURORA-174 could probably be closed with this?(since the rack limit
constraint can be configured in the .aurora file)
I can't really estimate the effect of my changes in
StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look
at the changes I did there.
Since this is also my first code submit, comments about codestyleother bad
habbits are very appreciated.
Diffs
-
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
5dfbcf1f6de716502a28f7da33a095968eb8420e
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
92ba45033ada8114349c435316c9681395aea706
Diff: https://reviews.apache.org/r/30010/diff/
Testing
---
Added test for ConfigurationManager.hasName
Added test testNoHostAndRackConstraintsAdded, that checks if the constraints
are present
Tested on vagrant devcluster to see if constraints are also gone in real
life
Thanks,
Florian Pfeiffer
Review Request 30293: Updating release script to work with linked .auroraversion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30293/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- git add does not work with a symlinked file. Diffs - build-support/release/release-candidate fd217f8add548a3adffd8181a84cb99d6e943369 Diff: https://reviews.apache.org/r/30293/diff/ Testing --- ./build-support/release/release-candidate -r 1 Thanks, Maxim Khutornenko
Re: Review Request 30293: Updating release script to work with linked .auroraversion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30293/#review69705 --- Ship it! Ship It! - Kevin Sweeney On Jan. 26, 2015, 4:03 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30293/ --- (Updated Jan. 26, 2015, 4:03 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- git add does not work with a symlinked file. Diffs - build-support/release/release-candidate fd217f8add548a3adffd8181a84cb99d6e943369 Diff: https://reviews.apache.org/r/30293/diff/ Testing --- ./build-support/release/release-candidate -r 1 Thanks, Maxim Khutornenko
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 27, 2015, 1:08 vorm.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 5dfbcf1f6de716502a28f7da33a095968eb8420e src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 92ba45033ada8114349c435316c9681395aea706 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/#review69594 --- Master (7ba6226) is red with this patch. ./build-support/jenkins/build.sh :assemble :compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7' 1 warning :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava :processTestResources :testClasses :checkstyleTest :findbugsJmh :findbugsMain :findbugsTest :licenseJmh UP-TO-DATE :licenseMain UP-TO-DATE :licenseTest UP-TO-DATE :license UP-TO-DATE :pmdMain :test :jacocoTestReport Coverage report generated: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html :analyzeReport Instruction coverage of 0.8917038316949588 exceeds minimum coverage of 0.89. :analyzeReport FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':analyzeReport'. Branch coverage is 0.8343558282208589, but must be greater than 0.835 * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 3 mins 23.668 secs I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Jan. 25, 2015, 8:10 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 25, 2015, 8:10 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 5dfbcf1f6de716502a28f7da33a095968eb8420e src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 92ba45033ada8114349c435316c9681395aea706 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/#review69688 --- Looks great to me, thanks for retaining the legacy behavior! I'm good to give a ship once these small nits are addressed. src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/30010/#comment114412 Please flip this to act in the affirmative. Arguments that act as double-negative are confusing, especially at 3 AM while a cluster is on fire. src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/30010/#comment114417 s/public // src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/30010/#comment114418 empty line between javadoc body and tags src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/30010/#comment114419 s/public // src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java https://reviews.apache.org/r/30010/#comment114420 Comment style should be a space between comment characters and comment text, and always in a complete sentence with punctuation. In this case, the line just needs the space at the front and a period at the end. There are a few lines needing this treatment in the test as well. src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java https://reviews.apache.org/r/30010/#comment114421 'By default, legacy constraints are applied to production services.' src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java https://reviews.apache.org/r/30010/#comment114423 We typically don't do this: ``` //isLegacyDisabled=true ``` Ideally we could do this in code with named parameters, but alas the language does not permit it. I propose removing the comment since it is not immune to refactors, and deviates from the rest of the codebase. - Bill Farner On Jan. 25, 2015, 8:10 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 25, 2015, 8:10 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 5dfbcf1f6de716502a28f7da33a095968eb8420e src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 92ba45033ada8114349c435316c9681395aea706 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Re: Review Request 30286: Raise .auroraversion to 0.7.0-SNAPSHOT
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30286/#review69694 --- This patch does not apply cleanly on master (ff8cdcf), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Jan. 26, 2015, 9:27 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30286/ --- (Updated Jan. 26, 2015, 9:27 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Bumping up aurora snapshot version in preparation for the 0.7.0 release. Diffs - src/main/resources/apache/aurora/client/cli/.auroraversion e8ff9d45c6326c28dae836a1409ec0c9b00fd75a Diff: https://reviews.apache.org/r/30286/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 26, 2015, 11:42 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Changes --- People -= Steve People += Maxim Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30293: Updating release script to work with linked .auroraversion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30293/ --- (Updated Jan. 27, 2015, 12:10 a.m.) Review request for Aurora and Kevin Sweeney. Changes --- -wfarner +kevints Repository: aurora Description --- git add does not work with a symlinked file. Diffs - build-support/release/release-candidate fd217f8add548a3adffd8181a84cb99d6e943369 Diff: https://reviews.apache.org/r/30293/diff/ Testing --- ./build-support/release/release-candidate -r 1 Thanks, Maxim Khutornenko
Re: Review Request 30285: Add Protobufs anonymous class back to untested classes list.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30285/ --- (Updated Jan. 26, 2015, 9:05 p.m.) Review request for Aurora and Zameer Manji. Repository: aurora Description (updated) --- This broke as a result of me reverting a change done in https://reviews.apache.org/r/28920/ when resolving merge conflicts. Prior to that patch, a call to `LOG.fine` was guarded by an if statement, which was removed. I reverted that since the purpose of the guard was performance-related (protobuf's `toString()` function can be very expensive). For some unknown reason my local builds continued to pass after this change, despite failing in jenkins. This was made worse by https://issues.apache.org/jira/browse/AURORA-1060, which caused review bot to fail at a point before this check was done. Diffs - config/legacy_untested_classes.txt 3429e35376cf71863dcc63c9c667aa70b0ec6b22 Diff: https://reviews.apache.org/r/30285/diff/ Testing --- Thanks, Bill Farner
Review Request 30286: Raise .auroraversion to 0.7.0-SNAPSHOT
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30286/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Bumping up aurora snapshot version in preparation for the 0.7.0 release. Diffs - src/main/resources/apache/aurora/client/cli/.auroraversion e8ff9d45c6326c28dae836a1409ec0c9b00fd75a Diff: https://reviews.apache.org/r/30286/diff/ Testing --- Thanks, Maxim Khutornenko
