Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check

2014-06-04 Thread Kevin Sweeney 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22168/#review44784
---

Ship it!


Ship It!

- 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 
>   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 
> a71f1eb7ac95

Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check

2014-06-04 Thread Brian Wickman 


> On June 4, 2014, 5:10 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, lines 36-41
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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 
> 8b29f3

Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check

2014-06-04 Thread Kevin Sweeney 

---
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


Rather than exclude these from checks I'd argue for conformance.



src/main/python/apache/aurora/client/bin/aurora_client.py


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


Looks like a bug in twitter checkstyle, did you open an issue?



src/main/python/apache/aurora/client/cli/__init__.py


registered_nouns



src/main/python/apache/aurora/client/cli/__init__.py


If register_nouns was annotated as abstract would this pass?



src/main/python/apache/aurora/client/cli/cron.py


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


Does this break help?



src/main/python/apache/aurora/common/auth/__init__.py


Another checkstyle bug? Would using .__name__ instead make sense here?



src/main/python/apache/aurora/executor/bin/thermos_runner_main.py


a comment explaining the magic here would be useful



src/main/python/apache/aurora/tools/java/organize_imports.py


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/ta

Re: Review Request 22168: Make style consistent with build-support/python/checkstyle-check

2014-06-04 Thread Brian Wickman 

---
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 
> a71f1eb7ac95a111