> 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 
>   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 
> 38a4c1d99a5f8b2fb57f2099a32d772e9eccfa56 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 05a89eed5d7933ef0c02f592cee6a086365a9a7f 
>   src/main/python/apache/aurora/executor/common/kill_manager.py 
> 2bfb5dc566ac64789c661bd9629546fe0a451ca2 
>   src/main/python/apache/aurora/executor/common/sandbox.py 
> b4125f43790027e9a2ac0ee2ee8eece87199e3a0 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 1800ee76b22b3a0f45c437db526035c26be24a3c 
>   src/main/python/apache/aurora/executor/thermos_runner.py 
> aef45d73bf1cbe52a3cf658c35e4f6464f46c761 
>   src/main/python/apache/aurora/tools/java/organize_imports.py 
> f5472ecd0bb90c8ea1066368b3f825612a33cbcc 
>   src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py 
> ea9a482b2f92efad0fd5c51a622d533826cabd48 
>   src/main/python/apache/thermos/bin/thermos.py 
> 196dfae1904f4495647632665dc2ae107b11aca7 
>   src/main/python/apache/thermos/bin/thermos_ckpt.py 
> a8e42bce9e4f1fc28397212dc06945e1ce334c2d 
>   src/main/python/apache/thermos/common/ckpt.py 
> af971f5e872b07dec3673ed2bec1d31db33f53c7 
>   src/main/python/apache/thermos/common/options.py 
> f3debc3a2a7d0e937a0a3ce0bfd25f830b63ebb0 
>   src/main/python/apache/thermos/common/path.py 
> 027816166444ab86dca22bd99ecc306eff74961f 
>   src/main/python/apache/thermos/common/planner.py 
> 58bd68d4ca7ad6a696d48cc73e5456220597d46e 
>   src/main/python/apache/thermos/config/bin/config_load.py 
> a0b4800721792da43e6505649cb35fcfc127efdf 
>   src/main/python/apache/thermos/config/bin/config_repl.py 
> 8776f9c17a7f2f780cc410a9c1eeb604d2670222 
>   src/main/python/apache/thermos/config/loader.py 
> 160027aecd68882086e37ff3b2e8ce9e59344512 
>   src/main/python/apache/thermos/config/schema.py 
> ca16c575384926aa4764f6b98bbc740b82e5d098 
>   src/main/python/apache/thermos/config/schema_base.py 
> 5fc77a8d88b8be63f5f625c93ce75ac0aa0df843 
>   src/main/python/apache/thermos/config/schema_helpers.py 
> 38620f7ea8280fe08c75bc7accdb10b6ca9180ce 
>   src/main/python/apache/thermos/core/inspector.py 
> f85b5f962dd14792636161b7bd52058f25119b57 
>   src/main/python/apache/thermos/core/muxer.py 
> ea2665cf394ebe4affdeff3034bd7d4715c86823 
>   src/main/python/apache/thermos/core/process.py 
> aef1ec6632325b8f7386cbe34f5c97d71371937f 
>   src/main/python/apache/thermos/core/runner.py 
> b04c55500af359bdd05654c0997c47d37ba1d524 
>   src/main/python/apache/thermos/monitoring/BUILD 
> 1ae5f520814b56369ad3d3188de30650481b4d1c 
>   src/main/python/apache/thermos/monitoring/detector.py 
> fa50fceeea72865b74e50649c693f564b3b2ffb5 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 23c1a93c703d0707fefed2d4203eac5db2b28326 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 11b8c764db780eed930c588d6ccad59b52749586 
>   src/main/python/apache/thermos/monitoring/process.py 
> 99f7a3db58cde203ee9de3112722268b704fcffa 
>   src/main/python/apache/thermos/monitoring/process_collector_psutil.py 
> f972c8349c829d5320a0d5cbd92b64d8ce9def22 
>   src/main/python/apache/thermos/monitoring/resource.py 
> 9f8e24e917c74587a3477182194a27cefcc3e2ee 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> c205f149ef0d0b3eb84b71e1c2b7dc6685b7b005 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 6b53c8671175554c7116f28ef474f82cad0c880a 
>   src/main/python/apache/thermos/observer/http/http_observer.py 
> 2bd079c9f2f7ab17da30b14a96bede5995d8a025 
>   src/main/python/apache/thermos/observer/http/static_assets.py 
> be881bf8a76058a95dad9acc250774cc1827c64e 
>   src/main/python/apache/thermos/testing/runner.py 
> 551311f780b8c6df4f105ca50a28cb96b568e694 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> ed0782b72eb12c45cefda2c3081f48b933dda206 
>   src/test/python/apache/aurora/client/api/test_disambiguator.py 
> fb6380d3f20c5f1339efdf9cb235507512c2d79c 
>   src/test/python/apache/aurora/client/api/test_health_check.py 
> 52cfcfedb229ae78de59a1976d14cb01c07e65e4 
>   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_quota_check.py 
> 6c241b788f83f86f6612022d5fee354d84ae5d40 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> e31fd6d4f9940cefdf86a1f39d44ad8bcca17690 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 402b426c852672c9351ac39f67061e553ed22a21 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 9c3bb6d386abbdbbfa8746639798ddca01d54fe9 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> ba783da7c0d93bb0bfd03809f62ddcad3f98cd0a 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 1ee11ff064b7d8c5af915482de191ea045592c1d 
>   src/test/python/apache/aurora/client/cli/test_config_noun.py 
> 7a9d733835e204bfd179e698bc335435b837d24b 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> b186b52416a2fae8de28fd1d21e7eec07fea8e55 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> 049405a4323fc73102d6ac1dae2230be4325c530 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 666ec3aa0745191aa1395e47728343cd0eda7115 
>   src/test/python/apache/aurora/client/cli/test_open.py 
> 5557c35c681ff17e0a5d0b4c79b2564211003ca8 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> 00b45838aee3f43caf81fedd9f88f57c04755229 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 50acc09491ac21935af78499ad66726df5a8f2ff 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 26633981d561eeed498acece03c025bec00e11bd 
>   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_admin_sla.py 
> 84a91d55516594dadc01408eb9d0b4773060d1af 
>   src/test/python/apache/aurora/client/commands/test_cancel_update.py 
> f8df6ae1e9b9b4a63010ca8b65406dcb68b2fd3d 
>   src/test/python/apache/aurora/client/commands/test_create.py 
> 75f068250b31b656c9c87a6aa66872fbb777b0c0 
>   src/test/python/apache/aurora/client/commands/test_hooks.py 
> 7648bf4869d0d0b029de1ee4f6de4f62b1b1315e 
>   src/test/python/apache/aurora/client/commands/test_kill.py 
> 3e2ac1fcea301f0ae986b61d9851d10e86996a20 
>   src/test/python/apache/aurora/client/commands/test_listjobs.py 
> 69dd6b42e8ce7672be44e1036d6a022cde6c97d0 
>   src/test/python/apache/aurora/client/commands/test_restart.py 
> 6e0159f134388a251cb44cd700102d05467a9062 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> cb053d1560476a8a93c8abdd5552532b56985543 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> c5afbd33d1b2f82e9603c93b967fbc942c0952d7 
>   src/test/python/apache/aurora/client/test_binding_helper.py 
> cc9397b9a77b229af294e6d2c56639cdebad5904 
>   src/test/python/apache/aurora/client/test_config.py 
> 4b3e1499c7447792288f4b480cbd756c8e19a831 
>   src/test/python/apache/aurora/common/test_cluster.py 
> 8559dfc75955da8b8fd63e0989f7be706f069925 
>   src/test/python/apache/aurora/common/test_cluster_option.py 
> af671eef15154a1f74c8816e13fad88473269852 
>   src/test/python/apache/aurora/common/test_clusters.py 
> 2f5632f802a9f4584cb0f8a32b97e3be2cb47b66 
>   src/test/python/apache/aurora/common/test_http_signaler.py 
> 6bac80afa9f842d68b40e96d6a16cb1029e1adc8 
>   src/test/python/apache/aurora/common/test_shellify.py 
> 6bdecb7dad5dc9b4d70c488c98aa0e49c9106ebc 
>   src/test/python/apache/aurora/config/test_base.py 
> 72a711a61fed246cdb7614d4ea95a7fd303e1562 
>   src/test/python/apache/aurora/config/test_constraint_parsing.py 
> 385f6a338c8463bf6747ff52cda7d5d8a6a6dffc 
>   src/test/python/apache/aurora/config/test_loader.py 
> adde41babcce8b97a26fe31daff36671e15c2f4a 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 9fea33d71bcd669384a2ccb7d2a52e4daf21b2ab 
>   src/test/python/apache/aurora/executor/common/test_directory_sandbox.py 
> 4d62cf93a2489054ebd5cbf44cff35e44afdbf7e 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 4071b40eb7088a0c4e8027a5879bf580cc89ba2e 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 
> 394ee71e5676f21661005398f27f8b444c5c8984 
>   src/test/python/apache/aurora/executor/test_executor_vars.py 
> b35387f20179a4f3924d7ab64c90817f6b770ee1 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> e98bffa872098459b92815e5ecac30ff3262c8ba 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> b28fc32f2c21aa573c7b38b8bee380c29be51a94 
>   src/test/python/apache/thermos/bin/test_thermos.py 
> 67fa90b6b770aa9c53d79294c373fb50430cd9e4 
>   src/test/python/apache/thermos/common/test_planner.py 
> f8cf49bec332e28c47a64b54e6c8fef59a5b327d 
>   src/test/python/apache/thermos/common/test_task_planner.py 
> ffdc30bbecb79a66bad052f634f107892cb07a7a 
>   src/test/python/apache/thermos/config/test_schema.py 
> f48f6c6d2c17ba21e0357f793fa8c9bf5cec1cd6 
>   src/test/python/apache/thermos/core/test_angry.py 
> 02280ed0ae20fa2cdf8a5ac267779ff51df93df6 
>   src/test/python/apache/thermos/core/test_ephemerals.py 
> 34b29aaa78e8331c5465e395d86c886bb7b4ab9d 
>   src/test/python/apache/thermos/core/test_failing_runner.py 
> 0c4e812a4d1902c6003aa6f08ea7e893915d36d8 
>   src/test/python/apache/thermos/core/test_failure_limit.py 
> a37f44f8585993d9aa308299bf63c9d43c926a2f 
>   src/test/python/apache/thermos/core/test_finalization.py 
> 972785fa3ab1bb56b84f36a4fd36ef8ead834c59 
>   src/test/python/apache/thermos/core/test_helper.py 
> 26288fae94736825a8bf77b6b1681ad11a560756 
>   src/test/python/apache/thermos/core/test_process.py 
> ed66f7335b50675d418ec952b83b1523731ad539 
>   src/test/python/apache/thermos/core/test_runner_integration.py 
> fa84de791dd9355c3faa1ec95215b32c7ceeb8fc 
>   src/test/python/apache/thermos/core/test_staged_kill.py 
> e0a3d14d62d7d9227ba64c91469bac05aba621c7 
>   src/test/python/apache/thermos/monitoring/test_disk.py 
> cc3c20a1694da202da80e9eca57605f1c8849d03 
>   src/test/sh/org/apache/aurora/e2e/http_example.py 
> 04f7fa5c9bf36a36b395d870db02b97f8e5b6c66 
> 
> Diff: https://reviews.apache.org/r/22168/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python/apache/aurora:all -v
> ./pants src/test/python/apache/thermos:all -v
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Reply via email to