Re: Review Request 22457: Improve aurora job diff command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22457/#review58280 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/22457/#comment99224 This is no longer true. The `populated` field reprsents a single TaskConfig now. You'd need to inflate a task config list according to the local instance count. src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/22457/#comment99225 Please, refactor the correspondent method in util.py to give you a single scheduled task. There is nothing unique here that cannot be reused. src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/22457/#comment99228 A ScheduledTask does not have a `key` field. Please, avoid using dynamic properties as they are very confusing in test. src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/22457/#comment99226 These are not mocks anymore, please update method name. src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/22457/#comment99227 same here src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/22457/#comment99229 Missing `job` field as a future replacment to owner/environment/jobName. src/test/python/apache/aurora/client/cli/test_diff.py https://reviews.apache.org/r/22457/#comment99230 Is this still needed? If so, would be great to have a bit of formatting around it to really help debugging when needed. - Maxim Khutornenko On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22457/ --- (Updated Oct. 24, 2014, 2:50 p.m.) Review request for Aurora, Maxim Khutornenko and Brian Wickman. Bugs: aurora-520 https://issues.apache.org/jira/browse/aurora-520 Repository: aurora Description --- Add a new diff method, which uses field-by-field comparison of JSON trees for comparing running job configurations to potentially updated configs. - Allow exclusion of semantically irrelevant fields. - Provide a clearer list of the differences between configs. - Provide a scripting-friendly alternative JSON syntax for diffs. The old diff behavior is still available under the --use-shell-diff option. Diffs - src/main/python/apache/aurora/client/cli/BUILD 995570325bbb09ecbcc2ace5d223760c5d49367f src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 src/test/python/apache/aurora/client/cli/test_json_diff.py PRE-CREATION Diff: https://reviews.apache.org/r/22457/diff/ Testing --- New unit tests of the JSON tree diff code, plus a bunch of new job diff tests of the new functionality. All tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 22457: Improve aurora job diff command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22457/#review58294 --- Master (53f4e73) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.api.api . SUCCESS src.test.python.apache.aurora.client.api.disambiguator . SUCCESS src.test.python.apache.aurora.client.api.instance_watcher . SUCCESS src.test.python.apache.aurora.client.api.job_monitor . SUCCESS src.test.python.apache.aurora.client.api.mux . SUCCESS src.test.python.apache.aurora.client.api.quota_check . SUCCESS src.test.python.apache.aurora.client.api.restarter . SUCCESS src.test.python.apache.aurora.client.api.scheduler_client . SUCCESS src.test.python.apache.aurora.client.api.sla . SUCCESS src.test.python.apache.aurora.client.api.updater . SUCCESS src.test.python.apache.aurora.client.api.updater_util . SUCCESS src.test.python.apache.aurora.client.binding_helper . SUCCESS src.test.python.apache.aurora.client.cli.api . SUCCESS src.test.python.apache.aurora.client.cli.bridge . SUCCESS src.test.python.apache.aurora.client.cli.command_hooks . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.cli.help . SUCCESS src.test.python.apache.aurora.client.cli.inspect . SUCCESS src.test.python.apache.aurora.client.cli.job . FAILURE src.test.python.apache.aurora.client.config . SUCCESS - Aurora ReviewBot On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22457/ --- (Updated Oct. 24, 2014, 2:50 p.m.) Review request for Aurora, Maxim Khutornenko and Brian Wickman. Bugs: aurora-520 https://issues.apache.org/jira/browse/aurora-520 Repository: aurora Description --- Add a new diff method, which uses field-by-field comparison of JSON trees for comparing running job configurations to potentially updated configs. - Allow exclusion of semantically irrelevant fields. - Provide a clearer list of the differences between configs. - Provide a scripting-friendly alternative JSON syntax for diffs. The old diff behavior is still available under the --use-shell-diff option. Diffs - src/main/python/apache/aurora/client/cli/BUILD 995570325bbb09ecbcc2ace5d223760c5d49367f src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 src/test/python/apache/aurora/client/cli/test_json_diff.py PRE-CREATION Diff: https://reviews.apache.org/r/22457/diff/ Testing --- New unit tests of the JSON tree diff code, plus a bunch of new job diff tests of the new functionality. All tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 27129: Upgrade psutil to 2.1.3
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27129/#review58296 --- LGTM! Master (53f4e73) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 24, 2014, 2:13 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27129/ --- (Updated Oct. 24, 2014, 2:13 a.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-627 https://issues.apache.org/jira/browse/AURORA-627 Repository: aurora Description --- Upgrade psutil to 2.1.3 Diffs - 3rdparty/python/BUILD 4493bf1ec763f367d1549f84e9a627a35254bcc5 Diff: https://reviews.apache.org/r/27129/diff/ Testing --- e2e tests and on OS X 10.10 and from within the vagrant image: src.test.python.apache.aurora.admin.admin_util . SUCCESS src.test.python.apache.aurora.admin.host_maintenance . SUCCESS src.test.python.apache.aurora.client.api.api . SUCCESS src.test.python.apache.aurora.client.api.disambiguator . SUCCESS src.test.python.apache.aurora.client.api.instance_watcher . SUCCESS src.test.python.apache.aurora.client.api.job_monitor . SUCCESS src.test.python.apache.aurora.client.api.mux . SUCCESS src.test.python.apache.aurora.client.api.quota_check . SUCCESS src.test.python.apache.aurora.client.api.restarter . SUCCESS src.test.python.apache.aurora.client.api.scheduler_client . SUCCESS src.test.python.apache.aurora.client.api.sla . SUCCESS src.test.python.apache.aurora.client.api.updater . SUCCESS src.test.python.apache.aurora.client.api.updater_util . SUCCESS src.test.python.apache.aurora.client.binding_helper . SUCCESS src.test.python.apache.aurora.client.cli.api . SUCCESS src.test.python.apache.aurora.client.cli.bridge . SUCCESS src.test.python.apache.aurora.client.cli.command_hooks . SUCCESS src.test.python.apache.aurora.client.cli.config . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.cli.help . SUCCESS src.test.python.apache.aurora.client.cli.inspect . SUCCESS src.test.python.apache.aurora.client.cli.job . SUCCESS src.test.python.apache.aurora.client.cli.logging . SUCCESS src.test.python.apache.aurora.client.cli.plugins . SUCCESS src.test.python.apache.aurora.client.cli.quota . SUCCESS src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.commands.admin . SUCCESS src.test.python.apache.aurora.client.commands.core . SUCCESS src.test.python.apache.aurora.client.commands.hooks . SUCCESS src.test.python.apache.aurora.client.commands.maintenance . SUCCESS src.test.python.apache.aurora.client.commands.run . SUCCESS src.test.python.apache.aurora.client.commands.ssh . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS src.test.python.apache.aurora.client.hooks.hooked_api . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_aurora_job_key . SUCCESS
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58300 --- LGTM! Master (53f4e73) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 24, 2014, 1:50 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 1:50 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58299 --- Does it make sense to use the ReviewBoard Python client from rbtools? https://www.reviewboard.org/docs/rbtools/0.5/api/overview/ What is the plan to actually run this script? Is it safe to assume that it will be executed from an up to date git repo, or will it need to git pull? build-support/jenkins/review_feedback.py https://reviews.apache.org/r/27145/#comment99248 Our Python continuation indent style is generally the same as our Java style, so these should be indented 4 past the parent, not aligned? - Joshua Cohen On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 5:32 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 12:59 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Bill's feedback. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58339 --- src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java https://reviews.apache.org/r/27100/#comment99292 I don't understand what you mean by matches what we do for dedicated host mismatches as well. - Zameer Manji On Oct. 23, 2014, 6:50 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 23, 2014, 6:50 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 24, 2014, 8:09 p.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Changes --- rebase. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_listjobs.py:mock_options = Mock() src/test/python/apache/aurora/client/commands/test_listjobs.py: job = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_callback = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_wait = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = Mock() Diffs (updated) - src/test/python/apache/aurora/admin/test_admin_util.py f5c8c69c1109d15ee3886fb863014c3285240db1 src/test/python/apache/aurora/client/cli/test_command_hooks.py 60c75300501c36ac20a97f78ff18b3ca7af30699 src/test/python/apache/aurora/client/cli/test_cron.py c7b71c29d44150162fec8066947623fa91815424 src/test/python/apache/aurora/client/cli/test_diff.py
Re: Review Request 27129: Upgrade psutil to 2.1.3
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27129/#review58351 --- Ship it! Ship It! - Kevin Sweeney On Oct. 23, 2014, 7:13 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27129/ --- (Updated Oct. 23, 2014, 7:13 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-627 https://issues.apache.org/jira/browse/AURORA-627 Repository: aurora Description --- Upgrade psutil to 2.1.3 Diffs - 3rdparty/python/BUILD 4493bf1ec763f367d1549f84e9a627a35254bcc5 Diff: https://reviews.apache.org/r/27129/diff/ Testing --- e2e tests and on OS X 10.10 and from within the vagrant image: src.test.python.apache.aurora.admin.admin_util . SUCCESS src.test.python.apache.aurora.admin.host_maintenance . SUCCESS src.test.python.apache.aurora.client.api.api . SUCCESS src.test.python.apache.aurora.client.api.disambiguator . SUCCESS src.test.python.apache.aurora.client.api.instance_watcher . SUCCESS src.test.python.apache.aurora.client.api.job_monitor . SUCCESS src.test.python.apache.aurora.client.api.mux . SUCCESS src.test.python.apache.aurora.client.api.quota_check . SUCCESS src.test.python.apache.aurora.client.api.restarter . SUCCESS src.test.python.apache.aurora.client.api.scheduler_client . SUCCESS src.test.python.apache.aurora.client.api.sla . SUCCESS src.test.python.apache.aurora.client.api.updater . SUCCESS src.test.python.apache.aurora.client.api.updater_util . SUCCESS src.test.python.apache.aurora.client.binding_helper . SUCCESS src.test.python.apache.aurora.client.cli.api . SUCCESS src.test.python.apache.aurora.client.cli.bridge . SUCCESS src.test.python.apache.aurora.client.cli.command_hooks . SUCCESS src.test.python.apache.aurora.client.cli.config . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.cli.help . SUCCESS src.test.python.apache.aurora.client.cli.inspect . SUCCESS src.test.python.apache.aurora.client.cli.job . SUCCESS src.test.python.apache.aurora.client.cli.logging . SUCCESS src.test.python.apache.aurora.client.cli.plugins . SUCCESS src.test.python.apache.aurora.client.cli.quota . SUCCESS src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.commands.admin . SUCCESS src.test.python.apache.aurora.client.commands.core . SUCCESS src.test.python.apache.aurora.client.commands.hooks . SUCCESS src.test.python.apache.aurora.client.commands.maintenance . SUCCESS src.test.python.apache.aurora.client.commands.run . SUCCESS src.test.python.apache.aurora.client.commands.ssh . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS src.test.python.apache.aurora.client.hooks.hooked_api . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_aurora_job_key . SUCCESS src.test.python.apache.aurora.common.test_cluster . SUCCESS
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58301 --- Ship it! build-support/jenkins/review_feedback.py https://reviews.apache.org/r/27145/#comment99252 replace with ternary? build-support/jenkins/review_feedback.py https://reviews.apache.org/r/27145/#comment99253 Is there a legitimate case to allow non 200 response code here? build-support/jenkins/review_feedback.py https://reviews.apache.org/r/27145/#comment99254 Combine with the above print? - Maxim Khutornenko On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 5:32 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
On Oct. 24, 2014, 5:57 p.m., Joshua Cohen wrote: Does it make sense to use the ReviewBoard Python client from rbtools? https://www.reviewboard.org/docs/rbtools/0.5/api/overview/ What is the plan to actually run this script? Is it safe to assume that it will be executed from an up to date git repo, or will it need to git pull? I'm hesitant, mostly because i don't think the code savings is worth the cost of adding build infrastructure around this script. I could be swayed, though. On Oct. 24, 2014, 5:57 p.m., Joshua Cohen wrote: build-support/jenkins/review_feedback.py, lines 98-100 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line98 Our Python continuation indent style is generally the same as our Java style, so these should be indented 4 past the parent, not aligned? Fixed. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58299 --- On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 5:32 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58374 --- It's not obvious from the diff: where do we gain perf? Can you point to the place where we don't do store calls anymore? - Maxim Khutornenko On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 7:59 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/#review58376 --- Ship it! Ship It! - Zameer Manji On Oct. 24, 2014, 1:09 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 24, 2014, 1:09 p.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_listjobs.py: mock_options = Mock() src/test/python/apache/aurora/client/commands/test_listjobs.py: job = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_callback = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_wait = Mock() src/test/python/apache/aurora/client/commands/util.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = Mock()
Re: Review Request 27084: Fix error when job create is called with --open-browser.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27084/#review58377 --- Ship it! Ship It! - Zameer Manji On Oct. 23, 2014, 9:50 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27084/ --- (Updated Oct. 23, 2014, 9:50 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-886 https://issues.apache.org/jira/browse/aurora-886 Repository: aurora Description --- Fix error when job create is called with --open-browser. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 10f8e0d331ca607e55e3aa6f96014caea744ed9f src/test/python/apache/aurora/client/cli/test_create.py 328297ab1d29efb0adce8f4931a13929a04dcd9c Diff: https://reviews.apache.org/r/27084/diff/ Testing --- Added new test; all unit tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 27100: Cache the host's maintenance status with offer.
On Oct. 24, 2014, 2:10 p.m., Maxim Khutornenko wrote: It's not obvious from the diff: where do we gain perf? Can you point to the place where we don't do store calls anymore? If you notice in `SchedulingFilterImplTest.java` many of the tests no longer make calls to the maintenance controller. This is a performance gain because the maintenance controller makes a consistent read from the AttributeStore. Since this read is ommitted from every time we call the scheduling filter we gain performance. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58374 --- On Oct. 24, 2014, 12:59 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 12:59 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27145: Add a script that publishes build results to review board.
On Oct. 24, 2014, 5:57 p.m., Joshua Cohen wrote: Does it make sense to use the ReviewBoard Python client from rbtools? https://www.reviewboard.org/docs/rbtools/0.5/api/overview/ What is the plan to actually run this script? Is it safe to assume that it will be executed from an up to date git repo, or will it need to git pull? Bill Farner wrote: I'm hesitant, mostly because i don't think the code savings is worth the cost of adding build infrastructure around this script. I could be swayed, though. Yeah, I'm not sure it's worth it either, but figured it's worth mentioning. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58299 --- On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 5:32 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote: build-support/jenkins/review_feedback.py, line 39 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line39 Use print() as a function, here and throughout Done. On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote: build-support/jenkins/review_feedback.py, line 94 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line94 as we're on python2.7 you can use argparse now (which removes some boilerplate and gives nicer errors) Thanks, done. On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote: build-support/jenkins/review_feedback.py, line 97 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line97 read this from a file? argparse makes this easy Sure, done. On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote: build-support/jenkins/review_feedback.py, line 116 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line116 with argparse you can just do required=True Done, thanks again. On Oct. 24, 2014, 8:06 p.m., Kevin Sweeney wrote: build-support/jenkins/review_feedback.py, line 149 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line149 Would the correct Apache exclaimation be +1 here? Sure, done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58338 --- On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 5:32 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
On Oct. 24, 2014, 8:57 p.m., Maxim Khutornenko wrote: build-support/jenkins/review_feedback.py, lines 37-38 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line37 replace with ternary? Done. On Oct. 24, 2014, 8:57 p.m., Maxim Khutornenko wrote: build-support/jenkins/review_feedback.py, line 45 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line45 Is there a legitimate case to allow non 200 response code here? 201 for POST. On Oct. 24, 2014, 8:57 p.m., Maxim Khutornenko wrote: build-support/jenkins/review_feedback.py, line 47 https://reviews.apache.org/r/27145/diff/1/?file=732311#file732311line47 Combine with the above print? Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58301 --- On Oct. 24, 2014, 5:32 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 5:32 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 9:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs (updated) - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27084: Fix error when job create is called with --open-browser.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27084/#review58387 --- Ship it! Ship It! - David McLaughlin On Oct. 23, 2014, 4:50 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27084/ --- (Updated Oct. 23, 2014, 4:50 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-886 https://issues.apache.org/jira/browse/aurora-886 Repository: aurora Description --- Fix error when job create is called with --open-browser. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 10f8e0d331ca607e55e3aa6f96014caea744ed9f src/test/python/apache/aurora/client/cli/test_create.py 328297ab1d29efb0adce8f4931a13929a04dcd9c Diff: https://reviews.apache.org/r/27084/diff/ Testing --- Added new test; all unit tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58391 --- Ship it! Can you document your exit codes somewhere? - Zameer Manji On Oct. 24, 2014, 2:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 2:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58392 --- +1: Master (5be667f) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 24, 2014, 9:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 9:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58396 --- Ship it! Ship It! - Joshua Cohen On Oct. 24, 2014, 9:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 9:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58400 --- +1: Master (5be667f) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 7:59 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/#review58403 --- -1: Master (5be667f) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.api.updater_util . SUCCESS src.test.python.apache.aurora.client.binding_helper . SUCCESS src.test.python.apache.aurora.client.cli.api . SUCCESS src.test.python.apache.aurora.client.cli.bridge . SUCCESS src.test.python.apache.aurora.client.cli.command_hooks . SUCCESS src.test.python.apache.aurora.client.cli.config . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.cli.help . SUCCESS src.test.python.apache.aurora.client.cli.inspect . SUCCESS src.test.python.apache.aurora.client.cli.job . SUCCESS src.test.python.apache.aurora.client.cli.logging . SUCCESS src.test.python.apache.aurora.client.cli.plugins . SUCCESS src.test.python.apache.aurora.client.cli.quota . SUCCESS src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.commands.admin . SUCCESS src.test.python.apache.aurora.client.commands.core . FAILURE src.test.python.apache.aurora.client.config . SUCCESS - Aurora ReviewBot On Oct. 24, 2014, 8:09 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 24, 2014, 8:09 p.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (
Re: Review Request 27100: Cache the host's maintenance status with offer.
On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote: src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java, line 119 https://reviews.apache.org/r/27100/diff/1/?file=730205#file730205line119 I don't understand what you mean by matches what we do for dedicated host mismatches as well. Here's the code snippet: if (!ConfigurationManager.isDedicated(task) isDedicated(slaveHost)) { return ImmutableSet.of(DEDICATED_HOST_VETO); } return ImmutableSet.Vetobuilder() .addAll(getConstraintFilter(attributeAggregate, slaveHost).apply(task)) .addAll(getResourceVetoes(offer, task)) .addAll(getMaintenanceVeto(slaveHost).asSet()) .build(); If the host is a known completely bad fit, we don't look at it further. I believe the same flow could be done for maintenance veteos, but at the caller rather than in `SchedulingFilterImpl`. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58339 --- On Oct. 24, 2014, 7:59 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 7:59 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27114: Move from github to bintray for pants support binaries.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27114/#review58409 --- Ship it! Thanks! This is now on master: $ git log -1 origin/master --abbrev-commit commit f98bec7 Author: Joe Smith yasumo...@gmail.com Date: Fri Oct 24 15:13:21 2014 -0700 Move from github to bintray for pants support binaries. Reviewed at https://reviews.apache.org/r/27114/ - Bill Farner On Oct. 23, 2014, 10:19 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27114/ --- (Updated Oct. 23, 2014, 10:19 p.m.) Review request for Aurora, John Sirois, Kevin Sweeney, and Bill Farner. Repository: aurora Description --- Move from github to bintray for pants support binaries. This is from advice from jsirois on https://groups.google.com/d/msg/pants-devel/khP3TuSqWmo/nLqgc671Og0J Diffs - pants.ini 6f49f94236d1e1ed721578365173c96dc4420b02 Diff: https://reviews.apache.org/r/27114/diff/ Testing --- [tw-mbp13-jsmith aurora (yasumoto/pants_bintray)]$ ./pants ./src/test/python/apache/aurora/admin:all Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/admin/BUILD, all)]) test session starts = platform darwin -- Python 2.7.6 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 11 items src/test/python/apache/aurora/admin/test_host_maintenance.py ... = 11 passed in 0.62 seconds == test session starts = platform darwin -- Python 2.7.6 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 2 items src/test/python/apache/aurora/admin/test_admin_util.py .. == 2 passed in 0.19 seconds == src.test.python.apache.aurora.admin.admin_util . SUCCESS src.test.python.apache.aurora.admin.host_maintenance . SUCCESS Thanks, Joe Smith
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 3:38 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Bill's feedback. The approach is different than suggested after talking with Bill offline. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27145: Add a script that publishes build results to review board.
On Oct. 24, 2014, 2:49 p.m., Zameer Manji wrote: Can you document your exit codes somewhere? Bill Farner wrote: Is that worthwhile? If it means documenting, i'd rather not vary them at all. I don't think this is something that should be scripted against. I asumed you varried them for a reason. If you are not scripting against them then just do sys.exit(1) otherwise it is confusing IMHO. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58391 --- On Oct. 24, 2014, 2:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 2:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58415 --- Ship it! src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java https://reviews.apache.org/r/27100/#comment99410 // TODO(wfarner): Replace this with HostAttributes for more use of this caching. src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java https://reviews.apache.org/r/27100/#comment99412 maintenanceVeto.isPresent() - Bill Farner On Oct. 24, 2014, 10:38 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 10:38 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27145: Add a script that publishes build results to review board.
On Oct. 24, 2014, 9:49 p.m., Zameer Manji wrote: Can you document your exit codes somewhere? Bill Farner wrote: Is that worthwhile? If it means documenting, i'd rather not vary them at all. I don't think this is something that should be scripted against. Zameer Manji wrote: I asumed you varried them for a reason. If you are not scripting against them then just do sys.exit(1) otherwise it is confusing IMHO. Agreed, done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58391 --- On Oct. 24, 2014, 9:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 9:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 10:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs (updated) - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 24, 2014, 10:48 p.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Changes --- Thanks ReviewBot! I forgot to run the commands target rather than just the cli one. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_listjobs.py:mock_options = Mock() src/test/python/apache/aurora/client/commands/test_listjobs.py: job = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_callback = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_wait = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = Mock() Diffs (updated) - src/test/python/apache/aurora/admin/test_admin_util.py f5c8c69c1109d15ee3886fb863014c3285240db1 src/test/python/apache/aurora/client/cli/test_command_hooks.py 60c75300501c36ac20a97f78ff18b3ca7af30699 src/test/python/apache/aurora/client/cli/test_cron.py c7b71c29d44150162fec8066947623fa91815424
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/#review58420 --- Ship it! Long live ReviewBot! - Kevin Sweeney On Oct. 24, 2014, 3:48 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 24, 2014, 3:48 p.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_listjobs.py: mock_options = Mock() src/test/python/apache/aurora/client/commands/test_listjobs.py: job = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_callback = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_wait = Mock() src/test/python/apache/aurora/client/commands/util.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_api_factory =
Re: Review Request 27145: Add a script that publishes build results to review board.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/#review58423 --- +1: Master (f98bec7) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 24, 2014, 10:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27145/ --- (Updated Oct. 24, 2014, 10:42 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-883 https://issues.apache.org/jira/browse/AURORA-883 Repository: aurora Description --- I also removed `--stacktrace` from the gradle command. This makes for better tail output. I originally added `--stacktrace` to help debug build flakiness we were having due to jenkins machine configuration (but haven't had in a very long time): https://reviews.apache.org/r/23776/ Diffs - build-support/jenkins/build.sh 50ea950206b572f708feb0ebc1c3d04db6eaebd7 build-support/jenkins/review_feedback.py PRE-CREATION Diff: https://reviews.apache.org/r/27145/diff/ Testing --- I've run this a handful of times on my workstation, it seems to be doing its job. Thanks, Bill Farner
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58426 --- Ship it! Ship It! - Maxim Khutornenko On Oct. 24, 2014, 10:38 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 10:38 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27089: Add documentation for the beta-update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27089/ --- (Updated Oct. 24, 2014, 11:05 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: AURORA-763 https://issues.apache.org/jira/browse/AURORA-763 Repository: aurora Description --- Add documentation for the beta-update command. Diffs - docs/client-commands.md 0424e8f5b386e0342a4c06870bbcbc01c0f20323 Diff: https://reviews.apache.org/r/27089/diff/ Testing --- Rendered at https://github.com/wfarner/incubator-aurora/blob/wfarner/document_updater/docs/client-commands.md Thanks, Bill Farner
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 4:08 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Bill's feedback. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27100: Cache the host's maintenance status with offer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/#review58431 --- +1: Master (669981d) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 24, 2014, 11:08 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27100/ --- (Updated Oct. 24, 2014, 11:08 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-878 https://issues.apache.org/jira/browse/AURORA-878 Repository: aurora Description --- Cache the host's maintenance status with the offer. By caching the status alongside the offer the scheduler does not need to access the attribute store for every offer it considers for a task. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 92c843830df7a779abace38bb0ce84d4cbeb5af4 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 882cdfd9b79b262befb81437cbd9a31a6bc1e40f src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/http/Offers.java b7dfedae45bfbce8fb5890cd99fa5bd1879b8a36 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java c4435cb74925c9ed04a37820b22c3ecdfcad49d4 src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java ddd24c38cc13e0b53dfa6d07d8c42a4d498de1ec src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27100/diff/ Testing --- ./gradlew clean build -Pq Thanks, Zameer Manji
Re: Review Request 27089: Add documentation for the beta-update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27089/#review58432 --- Ship it! docs/client-commands.md https://reviews.apache.org/r/27089/#comment99430 Perhaps briefly mention the benefits of the server side updater and the upcoming removal of the client one? docs/client-commands.md https://reviews.apache.org/r/27089/#comment99427 missing closing '' - Maxim Khutornenko On Oct. 24, 2014, 11:05 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27089/ --- (Updated Oct. 24, 2014, 11:05 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: AURORA-763 https://issues.apache.org/jira/browse/AURORA-763 Repository: aurora Description --- Add documentation for the beta-update command. Diffs - docs/client-commands.md 0424e8f5b386e0342a4c06870bbcbc01c0f20323 Diff: https://reviews.apache.org/r/27089/diff/ Testing --- Rendered at https://github.com/wfarner/incubator-aurora/blob/wfarner/document_updater/docs/client-commands.md Thanks, Bill Farner
Review Request 27181: Allow the invoker to ignore patterns in git-clean.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Like, i dunno, the credentials file, for example. Diffs - build-support/jenkins/review_feedback.py 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 Diff: https://reviews.apache.org/r/27181/diff/ Testing --- Running. Thanks, Bill Farner
Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/#review58436 --- Ship it! build-support/jenkins/review_feedback.py https://reviews.apache.org/r/27181/#comment99437 Should've caught this in the first review - dashes instead of underscores, here and above. - Kevin Sweeney On Oct. 24, 2014, 4:19 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/ --- (Updated Oct. 24, 2014, 4:19 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Like, i dunno, the credentials file, for example. Diffs - build-support/jenkins/review_feedback.py 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 Diff: https://reviews.apache.org/r/27181/diff/ Testing --- Running. Thanks, Bill Farner
Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.
On Oct. 24, 2014, 11:21 p.m., Kevin Sweeney wrote: build-support/jenkins/review_feedback.py, line 112 https://reviews.apache.org/r/27181/diff/1/?file=733134#file733134line112 Should've caught this in the first review - dashes instead of underscores, here and above. Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/#review58436 --- On Oct. 24, 2014, 11:19 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/ --- (Updated Oct. 24, 2014, 11:19 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Like, i dunno, the credentials file, for example. Diffs - build-support/jenkins/review_feedback.py 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 Diff: https://reviews.apache.org/r/27181/diff/ Testing --- Running. Thanks, Bill Farner
Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/#review58438 --- -1: This patch does not apply cleanly on master (f98bec7), do you need to rebase? - Aurora ReviewBot On Oct. 24, 2014, 11:19 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/ --- (Updated Oct. 24, 2014, 11:19 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Like, i dunno, the credentials file, for example. Diffs - build-support/jenkins/review_feedback.py 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 Diff: https://reviews.apache.org/r/27181/diff/ Testing --- Running. Thanks, Bill Farner
Re: Review Request 27181: Allow the invoker to ignore patterns in git-clean.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27181/ --- (Updated Oct. 24, 2014, 11:25 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Like, i dunno, the credentials file, for example. Diffs (updated) - build-support/jenkins/review_feedback.py 049f2ceec4acc0a6901105d7c5fcccbbdbb90712 Diff: https://reviews.apache.org/r/27181/diff/ Testing --- Running. Thanks, Bill Farner
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/#review58444 --- -1: Master (3778330) is red with this patch. ./build-support/jenkins/build.sh Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.lang-0.3.0-py2.7-nspkg.pth Successfully installed twitter.checkstyle pyflakes pep8 GitPython twitter.common.app gitdb twitter.common.process twitter.common.log twitter.common.util twitter.common.collections async smmap twitter.common.string twitter.common.options twitter.common.dirutil twitter.common.contextutil twitter.common.lang Cleaning up... F401:ERROR src/test/python/apache/aurora/client/cli/test_task_run.py:024-031 'ScheduledTask' imported but unused |from gen.apache.aurora.api.ttypes import ( |JobKey, |ResponseCode, |ScheduledTask, |ScheduleStatus, |ScheduleStatusResult, |TaskQuery |) E501:ERROR src/test/python/apache/aurora/client/commands/test_maintenance.py:045-046 line too long (101 100 characters) |mock_options = Mock(spec_set=['cluster', 'disable_all_hooks', 'duration', 'filename', 'grouping', | 'hosts', 'percentage', 'post_drain_script', 'reason', 'unsafe_hosts_filename', 'verbosity']) - Aurora ReviewBot On Oct. 24, 2014, 10:48 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 24, 2014, 10:48 p.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py: mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py: mock_scheduler_proxy = Mock()
Review Request 27182: Add a test for the thermos resource module
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Add a test for the thermos resource module Diffs - src/main/python/apache/thermos/monitoring/monitor.py 8f87f5ffc39c87e87ff78b941ea30df7138bd1ef src/test/python/apache/thermos/monitoring/BUILD 33d6bba43aff6d62b2646491f004475c27ed99db src/test/python/apache/thermos/monitoring/test_resource.py PRE-CREATION Diff: https://reviews.apache.org/r/27182/diff/ Testing --- [tw-mbp13-jsmith aurora (yasumoto/psutil_2.1.3)]$ ./pants ./src/test/python/apache/thermos/monitoring:test_resource Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/thermos/monitoring/BUILD, test_resource)]) test session starts = platform darwin -- Python 2.7.6 -- py-1.4.26 -- pytest-2.6.4 plugins: cov, timeout collected 5 items src/test/python/apache/thermos/monitoring/test_resource.py . == 5 passed in 0.21 seconds == src.test.python.apache.thermos.monitoring.test_resource . SUCCESS Thanks, Joe Smith
Re: Review Request 27058: Add specs to instances of Mock in Python tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27058/ --- (Updated Oct. 25, 2014, 12:24 a.m.) Review request for Aurora, Mark Chu-Carroll and Zameer Manji. Changes --- Okay ran _all_ tests, and also ran isort-check and checkstyle-check again. Bugs: AURORA-248 https://issues.apache.org/jira/browse/AURORA-248 Repository: aurora Description --- Use of Mock() without a specification considered harmful. I went through and updated as many mocks as I could. Any remaining can be classified as: 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which uses __getattr__ to delegate to the read or write client. 2) Primitives like strings and callback functions or data objects like dicts and pystachio structs. 3) Weird mocks that broke code where they really shouldn't have (off the top of my head - in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were replaced with real thrift structs. The remaining offenders: $ grep -r --include=*.py Mock() src/test/python src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback = mock.Mock() src/test/python/apache/aurora/admin/test_host_maintenance.py:mock_callback = mock.Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_proxy = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_get = Mock() src/test/python/apache/aurora/client/api/test_api.py:mock_task_config = Mock() src/test/python/apache/aurora/client/api/test_job_monitor.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_quota_check.py: self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_scheduler_client.py: client._connect_scheduler = mock.MagicMock() src/test/python/apache/aurora/client/api/test_sla.py:self._scheduler = Mock() src/test/python/apache/aurora/client/api/test_task_util.py:scheduler = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/cli/test_inspect.py:raw_config = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_task = Mock() src/test/python/apache/aurora/client/cli/test_inspect.py:mock_process = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/test_kill.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_scheduler_client = Mock() src/test/python/apache/aurora/client/cli/util.py:mock_api_factory = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: job.assignedTask.task.executorConfig.data = Mock() src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_diff.py: patch('json.loads', return_value=Mock())) as ( src/test/python/apache/aurora/client/commands/test_listjobs.py:mock_options = Mock() src/test/python/apache/aurora/client/commands/test_listjobs.py: job = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_callback = Mock() src/test/python/apache/aurora/client/commands/test_maintenance.py: mock_wait = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_scheduler_proxy = Mock() src/test/python/apache/aurora/client/commands/util.py:mock_api_factory = Mock() Diffs (updated) - src/test/python/apache/aurora/admin/test_admin_util.py f5c8c69c1109d15ee3886fb863014c3285240db1 src/test/python/apache/aurora/client/cli/test_command_hooks.py 60c75300501c36ac20a97f78ff18b3ca7af30699 src/test/python/apache/aurora/client/cli/test_cron.py c7b71c29d44150162fec8066947623fa91815424
Review Request 27188: Use ship-it instead of +/-1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/ --- Review request for Aurora and Jake Farrell. Repository: aurora Description --- Use ship-it instead of +/-1. Diffs - build-support/jenkins/review_feedback.py 9d22358608d0d52019df0c4c5b96b08d0f157c43 Diff: https://reviews.apache.org/r/27188/diff/ Testing --- None yet, will do a dry run Monday before committing. Thanks, Bill Farner
Re: Review Request 27188: Use ship-it instead of +/-1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/#review58480 --- Ship it! Ship It! - Joe Smith On Oct. 24, 2014, 8:17 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/ --- (Updated Oct. 24, 2014, 8:17 p.m.) Review request for Aurora and Jake Farrell. Repository: aurora Description --- Use ship-it instead of +/-1. Diffs - build-support/jenkins/review_feedback.py 9d22358608d0d52019df0c4c5b96b08d0f157c43 Diff: https://reviews.apache.org/r/27188/diff/ Testing --- None yet, will do a dry run Monday before committing. Thanks, Bill Farner
Re: Review Request 27188: Use ship-it instead of +/-1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/#review58481 --- +1: Master (3778330) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Oct. 25, 2014, 3:17 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/ --- (Updated Oct. 25, 2014, 3:17 a.m.) Review request for Aurora and Jake Farrell. Repository: aurora Description --- Use ship-it instead of +/-1. Diffs - build-support/jenkins/review_feedback.py 9d22358608d0d52019df0c4c5b96b08d0f157c43 Diff: https://reviews.apache.org/r/27188/diff/ Testing --- None yet, will do a dry run Monday before committing. Thanks, Bill Farner
Re: Review Request 27188: Use ship-it instead of +/-1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/#review58483 --- Ship it! _~ _~)_)_~ )_))_))_) _!__!__!_ __t/ ~ - Joshua Cohen On Oct. 25, 2014, 3:17 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/ --- (Updated Oct. 25, 2014, 3:17 a.m.) Review request for Aurora and Jake Farrell. Repository: aurora Description --- Use ship-it instead of +/-1. Diffs - build-support/jenkins/review_feedback.py 9d22358608d0d52019df0c4c5b96b08d0f157c43 Diff: https://reviews.apache.org/r/27188/diff/ Testing --- None yet, will do a dry run Monday before committing. Thanks, Bill Farner
Re: Review Request 27188: Use ship-it instead of +/-1.
On Oct. 25, 2014, 4:39 a.m., Joshua Cohen wrote: _~ _~)_)_~ )_))_))_) _!__!__!_ __t/ ~ Wow did that ever not format properly! https://github.com/reviewboard/rb-extension-pack/blob/master/shipit_ascii_art/shipit_ascii_art/asciiart.py#L4 - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/#review58483 --- On Oct. 25, 2014, 3:17 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27188/ --- (Updated Oct. 25, 2014, 3:17 a.m.) Review request for Aurora and Jake Farrell. Repository: aurora Description --- Use ship-it instead of +/-1. Diffs - build-support/jenkins/review_feedback.py 9d22358608d0d52019df0c4c5b96b08d0f157c43 Diff: https://reviews.apache.org/r/27188/diff/ Testing --- None yet, will do a dry run Monday before committing. Thanks, Bill Farner