Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 4, 2014, 9:08 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Removed Bill from the reviewers list, since he's out getting married! Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py 2b8ea26e0362690cef38a1b907642d07aa1df37f src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 21440: Implementing parallel updater
On May 27, 2014, 6:38 p.m., Brian Wickman wrote: src/test/python/apache/aurora/client/api/test_instance_watcher.py, line 19 https://reviews.apache.org/r/21440/diff/4/?file=588632#file588632line19 I don't think we can rely upon patching or subclassing _Event (an internal implementation detail) for testing. it'd be better to just create duck-typed ClockEvent and NoWaitEvent with .wait and .is_set methods. these probably belong in twitter.common.testing but can be created a la carte for now. mba=science=; python3 Python 3.3.3 (default, Apr 15 2014, 11:17:36) [GCC 4.2.1 Compatible Apple Clang 4.0 ((tags/Apple/clang-421.0.60))] on darwin Type help, copyright, credits or license for more information. from threading import _Event Traceback (most recent call last): File stdin, line 1, in module ImportError: cannot import name _Event Maxim Khutornenko wrote: Done for the fakes. I was not able to make patch work with threading.Event though no matter what I tried (side_effect, return_value and etc.). Dropped a TODO to investigate a possible solution. can you file a JIRA and add it to the TODO? - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21440/#review44016 --- On May 30, 2014, 10:08 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21440/ --- (Updated May 30, 2014, 10:08 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-350 https://issues.apache.org/jira/browse/AURORA-350 Repository: aurora Description --- The updater now spawns upto batch_size threads to process one instance per thread. All mutating calls are multiplexed by the SchedulerMux to do batch kill/add/restart calls. This is the first step towards a fully multiplexed SchedulerProxy and is intended to mitigate LDAP/scheduler load. Diffs - src/main/python/apache/aurora/client/api/instance_watcher.py e09aa9a6c32c17f13c9b8ff3a589919587bd839b src/main/python/apache/aurora/client/api/job_monitor.py d176995fca68d42fcc2d3989483eaf520d0d737f src/main/python/apache/aurora/client/api/scheduler_client.py 7be974eb91089f776656ce65b64ee6d8c5b46394 src/main/python/apache/aurora/client/api/updater.py ea7285a75020a47142e1761c7ed455cdc838e37c src/main/python/apache/aurora/client/api/updater_util.py 04105de8fb2ce1cab049eb06fd313a43bdcd28db src/test/python/apache/aurora/client/api/test_instance_watcher.py b2d0c804ae2b2095d8d2a99ea42f4da06041cec8 src/test/python/apache/aurora/client/api/test_job_monitor.py 665db74475f4828af2050e98e20bbb3b1b29cf0c src/test/python/apache/aurora/client/api/test_updater.py ba783da7c0d93bb0bfd03809f62ddcad3f98cd0a src/test/python/apache/aurora/client/cli/test_create.py b186b52416a2fae8de28fd1d21e7eec07fea8e55 src/test/python/apache/aurora/client/cli/test_kill.py 666ec3aa0745191aa1395e47728343cd0eda7115 src/test/python/apache/aurora/client/cli/test_restart.py 50acc09491ac21935af78499ad66726df5a8f2ff src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 src/test/python/apache/aurora/client/commands/test_create.py 75f068250b31b656c9c87a6aa66872fbb777b0c0 src/test/python/apache/aurora/client/commands/test_kill.py 3e2ac1fcea301f0ae986b61d9851d10e86996a20 src/test/python/apache/aurora/client/commands/test_restart.py 6e0159f134388a251cb44cd700102d05467a9062 src/test/python/apache/aurora/client/commands/test_update.py c5afbd33d1b2f82e9603c93b967fbc942c0952d7 src/test/python/apache/aurora/client/commands/util.py 84784171816797f3a4fa4c5238d19b626e68ff44 src/test/python/apache/aurora/client/fake_scheduler_proxy.py 2a4773c81efb390385f675854e9631500b263a45 src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 80871273fc4d47558253e6b09c92724e8693bc11 src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora fc723cf232ddbc10458fc394e37358c8523118c2 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 9c5652829ac306dda5f7e95e164c85713e18988f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 2af256d65850bd86279dff4b5c53f234cf7a Diff: https://reviews.apache.org/r/21440/diff/ Testing --- ./pants src/test/python:all src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22168/#review44722 --- Kevin, ping. - Brian Wickman On June 3, 2014, 12:38 a.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22168/ --- (Updated June 3, 2014, 12:38 a.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes style to pass modified pep8 + flakes. (pep8 modifications are 100 col lines and 2-sp indents.) Diffs - src/main/python/apache/aurora/admin/host_maintenance.py ca26de1e1a75aa4ae1c47ddd0f566b577c67fc7c src/main/python/apache/aurora/client/api/__init__.py 1af0f8fa77e15444027c553404495d1ebeb5e540 src/main/python/apache/aurora/client/api/command_runner.py 8b29f35f85f4173158e0d46aba091915bf7200d0 src/main/python/apache/aurora/client/api/health_check.py d6ef596804a8d7c3261200475bd9b8e49fe27f22 src/main/python/apache/aurora/client/api/scheduler_client.py 7be974eb91089f776656ce65b64ee6d8c5b46394 src/main/python/apache/aurora/client/api/sla.py d15491affdffe51fa3a149ca5a80f7994eb5b35a src/main/python/apache/aurora/client/api/updater.py ea7285a75020a47142e1761c7ed455cdc838e37c src/main/python/apache/aurora/client/api/updater_util.py 04105de8fb2ce1cab049eb06fd313a43bdcd28db src/main/python/apache/aurora/client/base.py ef0855daf95b6bddb6788284102effde2599179b src/main/python/apache/aurora/client/bin/aurora_admin.py d1247e61f4f70955c18368331daf4905a2cc1134 src/main/python/apache/aurora/client/bin/aurora_client.py 1317fae66e9ceb45fa3654c2c09389d73ccfd868 src/main/python/apache/aurora/client/binding_helper.py d17d57e02ad1b6ebce6e955ddae6abd28d657cf9 src/main/python/apache/aurora/client/cli/__init__.py fd6f96ebe4acd358409f145178ebf8ad5ea27d05 src/main/python/apache/aurora/client/cli/bridge.py d5eec8aaebb9155c88c57fa76188fa9a8501b027 src/main/python/apache/aurora/client/cli/client.py 1fb5364894d230646592942434eb1da6554d5c05 src/main/python/apache/aurora/client/cli/command_hooks.py c349824f3c5bc156ebdbc46c1ea9093aa5a8634f src/main/python/apache/aurora/client/cli/config.py 034c68e52ac8d2fbabb9dbf4f397f1a4920d0122 src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/logsetup.py 15fb306873fb6dce2bba05546c407d59d40e26bf src/main/python/apache/aurora/client/cli/options.py 0d49bac2fa13ed5b156508a08a1af48c58582f8f src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py fe11f38b902ae54a4048ba114055ba30e8abe6c5 src/main/python/apache/aurora/client/commands/admin.py 919eea933a5a65396e64e05c739344f0c093c1b3 src/main/python/apache/aurora/client/commands/core.py 29e70a98585836c5208f1e058daa58ff8274090c src/main/python/apache/aurora/client/commands/help.py d59b2993912d362f11e92ead99e8dffc3b304d5c src/main/python/apache/aurora/client/commands/maintenance.py f6ebe3b0c665211f175f0b432a2fdae83fc7b62f src/main/python/apache/aurora/client/config.py 3b01792cae46c957424d7cdcc0d9ff954e29ae61 src/main/python/apache/aurora/client/factory.py 22805f0006ff9f7bd3efdc37f3686d6eea8d7417 src/main/python/apache/aurora/client/hooks/hooked_api.py a205777e29be9745a8ee8c89dc61372ffc3467ba src/main/python/apache/aurora/common/auth/__init__.py 95e56ad87d39aacb3031bf4c1408092bd252c335 src/main/python/apache/aurora/common/auth/auth_module.py aacb1b9fa2f05251b8ae44f3ac53177d7db14369 src/main/python/apache/aurora/common/cluster.py f04718f2bd1284dd3fb6f29b69d42bf8aeb76ebf src/main/python/apache/aurora/common/cluster_option.py 9ddd8686a3228c694472fe0f0684fcc3c9a9028b src/main/python/apache/aurora/common/clusters.py 389b6f9c2c860a25287eeb870a081ebde8a588e4 src/main/python/apache/aurora/common/http_signaler.py 4e5d7b44f90e3a72893941376600bb7754005a69 src/main/python/apache/aurora/config/loader.py 942f149bb117c7460e01f69f206ce24bd28f3106 src/main/python/apache/aurora/config/schema/base.py 43ae5cf72d6009ce33ad4aaa99980c40ea03f52c src/main/python/apache/aurora/config/thrift.py 419625df338fab8ef5e3840c4301d8c4f92a3d50 src/main/python/apache/aurora/executor/aurora_executor.py a71f1eb7ac95a1113109f5d18861319e6436c187 src/main/python/apache/aurora/executor/bin/thermos_runner_main.py
Re: Review Request 22233: Fix logging in the command-runner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22233/#review44723 --- src/main/python/apache/aurora/client/api/command_runner.py https://reviews.apache.org/r/22233/#comment79197 this should probably be logging.DEBUG if you really want to make it an instancemethod, it should go along with the other instancemethods below __init__. alternatively, it can stay a static/classmethod by passing a logger in 'args' (e.g. by tacking it onto the end of the list returned by self.process_arguments) - Brian Wickman On June 4, 2014, 2:56 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22233/ --- (Updated June 4, 2014, 2:56 p.m.) Review request for Aurora, David McLaughlin and Brian Wickman. Bugs: aurora-503 https://issues.apache.org/jira/browse/aurora-503 Repository: aurora Description --- Fix logging in the command-runner. Diffs - src/main/python/apache/aurora/client/api/__init__.py 1af0f8fa77e15444027c553404495d1ebeb5e540 src/main/python/apache/aurora/client/api/command_runner.py 8b29f35f85f4173158e0d46aba091915bf7200d0 src/main/python/apache/aurora/client/cli/client.py 1fb5364894d230646592942434eb1da6554d5c05 src/main/python/apache/aurora/client/cli/config.py 034c68e52ac8d2fbabb9dbf4f397f1a4920d0122 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/task.py fe11f38b902ae54a4048ba114055ba30e8abe6c5 src/test/python/apache/aurora/client/api/test_sla.py 9c3bb6d386abbdbbfa8746639798ddca01d54fe9 src/test/python/apache/aurora/client/cli/test_config_noun.py 7a9d733835e204bfd179e698bc335435b837d24b src/test/python/apache/aurora/client/cli/test_cron.py 049405a4323fc73102d6ac1dae2230be4325c530 src/test/python/apache/aurora/client/cli/test_task_run.py 26633981d561eeed498acece03c025bec00e11bd src/test/python/apache/aurora/client/test_config.py 4b3e1499c7447792288f4b480cbd756c8e19a831 src/test/sh/org/apache/aurora/e2e/http_example.py 04f7fa5c9bf36a36b395d870db02b97f8e5b6c66 Diff: https://reviews.apache.org/r/22233/diff/ Testing --- Added check to unit tests; all client tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22168/#review44679 --- src/main/python/apache/aurora/client/api/scheduler_client.py https://reviews.apache.org/r/22168/#comment79137 Rather than exclude these from checks I'd argue for conformance. src/main/python/apache/aurora/client/bin/aurora_client.py https://reviews.apache.org/r/22168/#comment79138 can this be combined into one line? from apache.aurora.client.commands import ( core, help as help_commands, run, ssh, ) src/main/python/apache/aurora/client/cli/__init__.py https://reviews.apache.org/r/22168/#comment79139 Looks like a bug in twitter checkstyle, did you open an issue? src/main/python/apache/aurora/client/cli/__init__.py https://reviews.apache.org/r/22168/#comment79140 registered_nouns src/main/python/apache/aurora/client/cli/__init__.py https://reviews.apache.org/r/22168/#comment79141 If register_nouns was annotated as abstract would this pass? src/main/python/apache/aurora/client/cli/cron.py https://reviews.apache.org/r/22168/#comment79142 Is this an isort limitation? I prefer the trailing comma since it lets me pipe through sort in my editor. src/main/python/apache/aurora/client/commands/help.py https://reviews.apache.org/r/22168/#comment79143 Does this break help? src/main/python/apache/aurora/common/auth/__init__.py https://reviews.apache.org/r/22168/#comment79144 Another checkstyle bug? Would using .__name__ instead make sense here? src/main/python/apache/aurora/executor/bin/thermos_runner_main.py https://reviews.apache.org/r/22168/#comment79145 a comment explaining the magic here would be useful src/main/python/apache/aurora/tools/java/organize_imports.py https://reviews.apache.org/r/22168/#comment79146 ws - Kevin Sweeney On June 2, 2014, 5:38 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22168/ --- (Updated June 2, 2014, 5:38 p.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes style to pass modified pep8 + flakes. (pep8 modifications are 100 col lines and 2-sp indents.) Diffs - src/main/python/apache/aurora/admin/host_maintenance.py ca26de1e1a75aa4ae1c47ddd0f566b577c67fc7c src/main/python/apache/aurora/client/api/__init__.py 1af0f8fa77e15444027c553404495d1ebeb5e540 src/main/python/apache/aurora/client/api/command_runner.py 8b29f35f85f4173158e0d46aba091915bf7200d0 src/main/python/apache/aurora/client/api/health_check.py d6ef596804a8d7c3261200475bd9b8e49fe27f22 src/main/python/apache/aurora/client/api/scheduler_client.py 7be974eb91089f776656ce65b64ee6d8c5b46394 src/main/python/apache/aurora/client/api/sla.py d15491affdffe51fa3a149ca5a80f7994eb5b35a src/main/python/apache/aurora/client/api/updater.py ea7285a75020a47142e1761c7ed455cdc838e37c src/main/python/apache/aurora/client/api/updater_util.py 04105de8fb2ce1cab049eb06fd313a43bdcd28db src/main/python/apache/aurora/client/base.py ef0855daf95b6bddb6788284102effde2599179b src/main/python/apache/aurora/client/bin/aurora_admin.py d1247e61f4f70955c18368331daf4905a2cc1134 src/main/python/apache/aurora/client/bin/aurora_client.py 1317fae66e9ceb45fa3654c2c09389d73ccfd868 src/main/python/apache/aurora/client/binding_helper.py d17d57e02ad1b6ebce6e955ddae6abd28d657cf9 src/main/python/apache/aurora/client/cli/__init__.py fd6f96ebe4acd358409f145178ebf8ad5ea27d05 src/main/python/apache/aurora/client/cli/bridge.py d5eec8aaebb9155c88c57fa76188fa9a8501b027 src/main/python/apache/aurora/client/cli/client.py 1fb5364894d230646592942434eb1da6554d5c05 src/main/python/apache/aurora/client/cli/command_hooks.py c349824f3c5bc156ebdbc46c1ea9093aa5a8634f src/main/python/apache/aurora/client/cli/config.py 034c68e52ac8d2fbabb9dbf4f397f1a4920d0122 src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/logsetup.py 15fb306873fb6dce2bba05546c407d59d40e26bf src/main/python/apache/aurora/client/cli/options.py 0d49bac2fa13ed5b156508a08a1af48c58582f8f src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py fe11f38b902ae54a4048ba114055ba30e8abe6c5
Re: Review Request 22092: Fixed syntax gotcha in .gitignore.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22092/#review44730 --- Ship it! Ship It! - Kevin Sweeney On June 3, 2014, 10:35 a.m., Suman Karumuri wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22092/ --- (Updated June 3, 2014, 10:35 a.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-485 https://issues.apache.org/jira/browse/AURORA-485 Repository: aurora Description --- Added a comment to .gitignore. Diffs - .gitignore bed5b8b8a02e957e05b8bcc8fed925f1432973cf Diff: https://reviews.apache.org/r/22092/diff/ Testing --- Thanks, Suman Karumuri
Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check
On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/api/scheduler_client.py, lines 36-41 https://reviews.apache.org/r/22168/diff/1/?file=602136#file602136line36 Rather than exclude these from checks I'd argue for conformance. we can't actually do that -- conformance would mean changing zk to ZK and zk_port to ZK_PORT, which would mean changing the field names in clusters.json On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/bin/aurora_client.py, lines 19-20 https://reviews.apache.org/r/22168/diff/1/?file=602142#file602142line19 can this be combined into one line? from apache.aurora.client.commands import ( core, help as help_commands, run, ssh, ) nope -- this is verboten by isort On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/__init__.py, line 100 https://reviews.apache.org/r/22168/diff/1/?file=602144#file602144line100 Looks like a bug in twitter checkstyle, did you open an issue? it is a bug -- but surmounting it would mean reimplementing pyflakes within the checkstyle tool. regardless, filed https://github.com/twitter/commons/issues/296 On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/__init__.py, line 325 https://reviews.apache.org/r/22168/diff/1/?file=602144#file602144line325 registered_nouns fixed On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/__init__.py, line 327 https://reviews.apache.org/r/22168/diff/1/?file=602144#file602144line327 If register_nouns was annotated as abstract would this pass? nope On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/cron.py, line 24 https://reviews.apache.org/r/22168/diff/1/?file=602150#file602150line24 Is this an isort limitation? I prefer the trailing comma since it lets me pipe through sort in my editor. this is done by isort. imho isort should prefer trailing commas. perhaps file an issue against isort. On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/commands/help.py, lines 50-51 https://reviews.apache.org/r/22168/diff/1/?file=602158#file602158line50 Does this break help? no -- checkstyle warns if you override builtins (def help() in this case). @app.command(name='help') will register the command in twitter.common.app as 'help' even though its function name is help_command On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/common/auth/__init__.py, lines 15-16 https://reviews.apache.org/r/22168/diff/1/?file=602163#file602163line15 Another checkstyle bug? Would using .__name__ instead make sense here? this is really a pyflakes bug. it should search __all__ and if a symbol is found in __all__, then consider it used. without the #noqa, pyflakes will warn that these variables are imported but unused. we could do AuthModule.__name__ but it seems non idiomatic. On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/tools/java/organize_imports.py, line 19 https://reviews.apache.org/r/22168/diff/1/?file=602179#file602179line19 ws fixed On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/executor/bin/thermos_runner_main.py, line 24 https://reviews.apache.org/r/22168/diff/1/?file=602173#file602173line24 a comment explaining the magic here would be useful this pattern is done in 9 different files -- it's probably better to document somewhere other than thermos_runner_main.py. open to suggestions. - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22168/#review44679 --- On June 3, 2014, 12:38 a.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22168/ --- (Updated June 3, 2014, 12:38 a.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes style to pass modified pep8 + flakes. (pep8 modifications are 100 col lines and 2-sp indents.) Diffs - src/main/python/apache/aurora/admin/host_maintenance.py ca26de1e1a75aa4ae1c47ddd0f566b577c67fc7c src/main/python/apache/aurora/client/api/__init__.py 1af0f8fa77e15444027c553404495d1ebeb5e540 src/main/python/apache/aurora/client/api/command_runner.py 8b29f35f85f4173158e0d46aba091915bf7200d0 src/main/python/apache/aurora/client/api/health_check.py d6ef596804a8d7c3261200475bd9b8e49fe27f22
Review Request 22242: Fixing query command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22242/ --- Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-301 https://issues.apache.org/jira/browse/AURORA-301 Repository: aurora Description --- Reordered the status check with result retrieval, which was hiding the query status message. Added some test coverage. Diffs - src/main/python/apache/aurora/client/commands/admin.py 919eea933a5a65396e64e05c739344f0c093c1b3 src/test/python/apache/aurora/client/commands/BUILD d5e497e491a0e526078b3389e9f194899f1795a2 src/test/python/apache/aurora/client/commands/test_admin.py PRE-CREATION Diff: https://reviews.apache.org/r/22242/diff/ Testing --- ./pants src/test/python/apache/aurora/client/commands:admin Thanks, Maxim Khutornenko
Review Request 22243: Variety of help fixes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22243/ --- Review request for Aurora, David McLaughlin and Brian Wickman. Bugs: aurora-437 and aurora-508 https://issues.apache.org/jira/browse/aurora-437 https://issues.apache.org/jira/browse/aurora-508 Repository: aurora Description --- Variety of help fixes. - Fixed glitch with string handling in aurora help noun verb. - Modified parameter name defaulting to produce better usage strings. - Added whitespace between lines of a noun's usage message. - Added missing parameter help messages for job list and job status parameters. Diffs - src/main/python/apache/aurora/client/cli/__init__.py fd6f96ebe4acd358409f145178ebf8ad5ea27d05 src/main/python/apache/aurora/client/cli/client.py 1fb5364894d230646592942434eb1da6554d5c05 src/main/python/apache/aurora/client/cli/config.py 034c68e52ac8d2fbabb9dbf4f397f1a4920d0122 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/options.py 0d49bac2fa13ed5b156508a08a1af48c58582f8f src/test/python/apache/aurora/client/api/test_sla.py 9c3bb6d386abbdbbfa8746639798ddca01d54fe9 src/test/python/apache/aurora/client/cli/test_config_noun.py 7a9d733835e204bfd179e698bc335435b837d24b src/test/python/apache/aurora/client/cli/test_cron.py 049405a4323fc73102d6ac1dae2230be4325c530 src/test/python/apache/aurora/client/test_config.py 4b3e1499c7447792288f4b480cbd756c8e19a831 src/test/sh/org/apache/aurora/e2e/http_example.py 04f7fa5c9bf36a36b395d870db02b97f8e5b6c66 Diff: https://reviews.apache.org/r/22243/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/#review44740 --- src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/22023/#comment79250 This pattern repeats making messageDEPRECATION removal harder. How about having something like: check_log_and_raise(self, resp, error) method that would do what check_and_log_response does but raise a custom error? src/main/python/apache/aurora/client/cli/cron.py https://reviews.apache.org/r/22023/#comment79251 A similar pattern but without raising an error. Perhaps something like check_and_log_custom_error(self, resp, context)? - Maxim Khutornenko On June 4, 2014, 1:08 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 4, 2014, 1:08 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py 2b8ea26e0362690cef38a1b907642d07aa1df37f src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/#review44744 --- src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/22023/#comment79263 I've got a strong preference to not do that. When I'm reading code, I find that things like that are a distraction: I can't see the control flow of the method; to understand how it works, I need to jump around the code. I also don't see much benefit in this case: in order to generate the error messages, you need to create the error anyway, and to do that, you need the message anyway. src/main/python/apache/aurora/client/cli/cron.py https://reviews.apache.org/r/22023/#comment79264 Here it's even harder to refactor as you suggest - this does a conditional return. To abstract this out, we'd need to switch to an exception throw, which would be more complex and less clear. - Mark Chu-Carroll On June 4, 2014, 9:08 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 4, 2014, 9:08 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py 2b8ea26e0362690cef38a1b907642d07aa1df37f src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 22092: Fixed syntax gotcha in .gitignore.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22092/ --- (Updated June 4, 2014, 8:35 p.m.) Review request for Aurora and Kevin Sweeney. Changes --- rebased with master Bugs: AURORA-485 https://issues.apache.org/jira/browse/AURORA-485 Repository: aurora Description --- Added a comment to .gitignore. Diffs (updated) - .gitignore 2d44c7001797e6f558039dcb8768905b64bc1e9a Diff: https://reviews.apache.org/r/22092/diff/ Testing --- Thanks, Suman Karumuri
