[
https://issues.apache.org/jira/browse/PHOENIX-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13902073#comment-13902073
]
Nick Dimiduk commented on PHOENIX-50:
-------------------------------------
[~jeffreyz] I think this is a good plan. Python is prevalent in our operating
environments and IMHO the benefit of single implementation trumps the
additional dependency. My only comment about jython is that the scripts will
suffer the startup time of the jvm.
For the scripts themselves, I think it's a good habit to follow the [PEP
8|http://www.python.org/dev/peps/pep-0008/] standard. There's a linter
available, {{pip install pep8}} will install it for you. Here's its output over
the scripts from your patch. Consider this my lazy-man's code review ;)
{noformat}
bin/csv-bulk-loader.py:26:80: E501 line too long (120 > 79 characters)
bin/csv-bulk-loader.py:33:80: E501 line too long (102 > 79 characters)
bin/csv-bulk-loader.py:34:80: E501 line too long (120 > 79 characters)
bin/csv-bulk-loader.py:37:10: E401 multiple imports on one line
bin/csv-bulk-loader.py:39:1: E302 expected 2 blank lines, found 1
bin/csv-bulk-loader.py:46:12: E225 missing whitespace around operator
bin/csv-bulk-loader.py:47:17: E225 missing whitespace around operator
bin/csv-bulk-loader.py:48:19: E225 missing whitespace around operator
bin/csv-bulk-loader.py:51:80: E501 line too long (130 > 79 characters)
bin/csv-bulk-loader.py:54:1: W391 blank line at end of file
bin/performance.py:25:10: E401 multiple imports on one line
bin/performance.py:27:1: E302 expected 2 blank lines, found 1
bin/performance.py:34:1: E302 expected 2 blank lines, found 1
bin/performance.py:34:46: E203 whitespace before ':'
bin/performance.py:38:1: E302 expected 2 blank lines, found 1
bin/performance.py:38:22: E203 whitespace before ':'
bin/performance.py:42:1: E302 expected 2 blank lines, found 1
bin/performance.py:42:12: E203 whitespace before ':'
bin/performance.py:43:1: E101 indentation contains mixed spaces and tabs
bin/performance.py:43:1: W191 indentation contains tabs
bin/performance.py:43:80: E501 line too long (98 > 79 characters)
bin/performance.py:44:1: E101 indentation contains mixed spaces and tabs
bin/performance.py:44:1: W191 indentation contains tabs
bin/performance.py:46:1: E302 expected 2 blank lines, found 1
bin/performance.py:46:45: E203 whitespace before ':'
bin/performance.py:51:21: E203 whitespace before ':'
bin/performance.py:56:10: E225 missing whitespace around operator
bin/performance.py:57:9: E225 missing whitespace around operator
bin/performance.py:58:6: E225 missing whitespace around operator
bin/performance.py:61:4: E225 missing whitespace around operator
bin/performance.py:62:5: E225 missing whitespace around operator
bin/performance.py:63:4: E225 missing whitespace around operator
bin/performance.py:64:11: E225 missing whitespace around operator
bin/performance.py:67:12: E225 missing whitespace around operator
bin/performance.py:68:17: E225 missing whitespace around operator
bin/performance.py:69:19: E225 missing whitespace around operator
bin/performance.py:70:22: E225 missing whitespace around operator
bin/performance.py:71:8: E225 missing whitespace around operator
bin/performance.py:74:80: E501 line too long (111 > 79 characters)
bin/performance.py:75:18: E225 missing whitespace around operator
bin/performance.py:77:80: E501 line too long (128 > 79 characters)
bin/performance.py:78:12: E121 continuation line indentation is not a multiple
of four
bin/performance.py:78:80: E501 line too long (81 > 79 characters)
bin/performance.py:82:12: E225 missing whitespace around operator
bin/performance.py:82:80: E501 line too long (92 > 79 characters)
bin/performance.py:83:80: E501 line too long (101 > 79 characters)
bin/performance.py:85:80: E501 line too long (120 > 79 characters)
bin/performance.py:98:80: E501 line too long (92 > 79 characters)
bin/performance.py:99:80: E501 line too long (97 > 79 characters)
bin/performance.py:100:80: E501 line too long (124 > 79 characters)
bin/performance.py:101:80: E501 line too long (93 > 79 characters)
bin/performance.py:115:1: W391 blank line at end of file
bin/psql.py:24:10: E401 multiple imports on one line
bin/psql.py:26:1: E302 expected 2 blank lines, found 1
bin/psql.py:33:12: E225 missing whitespace around operator
bin/psql.py:34:17: E225 missing whitespace around operator
bin/psql.py:35:19: E225 missing whitespace around operator
bin/psql.py:37:80: E501 line too long (111 > 79 characters)
bin/psql.py:38:80: E501 line too long (111 > 79 characters)
bin/psql.py:39:80: E501 line too long (119 > 79 characters)
bin/sqlline.py:24:10: E401 multiple imports on one line
bin/sqlline.py:26:1: E302 expected 2 blank lines, found 1
bin/sqlline.py:33:12: E225 missing whitespace around operator
bin/sqlline.py:34:17: E225 missing whitespace around operator
bin/sqlline.py:35:19: E225 missing whitespace around operator
bin/sqlline.py:37:21: E203 whitespace before ':'
bin/sqlline.py:38:80: E501 line too long (182 > 79 characters)
bin/sqlline.py:43:21: E203 whitespace before ':'
bin/sqlline.py:44:12: E225 missing whitespace around operator
bin/sqlline.py:44:35: E703 statement ends with a semicolon
bin/sqlline.py:46:80: E501 line too long (348 > 79 characters)
{noformat}
> Convert shell scripts under bin to python script so same scripts can run in
> unix & windows
> ------------------------------------------------------------------------------------------
>
> Key: PHOENIX-50
> URL: https://issues.apache.org/jira/browse/PHOENIX-50
> Project: Phoenix
> Issue Type: Improvement
> Reporter: Jeffrey Zhong
> Assignee: Jeffrey Zhong
> Fix For: 3.0.0
>
> Attachments: phoenix-50.patch
>
>
> Converting to python scripts so that we only need to maintain one version
> scripts for both linux like OS & windows.
> We had a test run of Phoenix against windows boxes and found few test
> failures. Later I'll check in fixes for those failure so that we can have
> Phoenix run on windows as well.
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
