You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@phoenix.apache.org by btbytes <gi...@git.apache.org> on 2016/07/29 03:37:06 UTC
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
GitHub user btbytes opened a pull request:
https://github.com/apache/phoenix/pull/186
cleanup sqlline-thin.py using python idioms.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/btbytes/phoenix master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/phoenix/pull/186.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #186
----
commit b4be67cc0b22f866f88ce2d56a38c48bf491160e
Author: Pradeep Gowda <pr...@btbytes.com>
Date: 2016-07-29T03:34:42Z
Cleanup sqlline-thin.py using python idioms.
commit 254a228e2ae9268069826be49aa6c3adf8b37c15
Author: Pradeep Gowda <pr...@btbytes.com>
Date: 2016-07-29T03:35:07Z
Merge branch 'master' of github.com:apache/phoenix
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
> quite a bit of this script is to poke the environment .. which is I/O. Unit testing may not yield much. But, i'll take a fresh look.
Yeah, I can understand where you're coming from. It would probably require some indirection to do it well. One concrete example I saw again was that we use JAVA_HOME from the environment unless it exists in the `hbase-env.*` script. These are the sorts of things that stand out at me that, long term, we should have tests to prove work as expected.
Again, I don't want to push them all on you in this changeset, but I'd be more than happy to work with someone who is a better python dev than myself :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
Left a few comments for you inline. Overall, I like the changes. Would love to see corresponding ones made for sqlline.py initially (we tend to make changes to one in the other as well). I don't think we would need to revisit all of the scripts for the first round (unless you're up to the task!).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on the issue:
https://github.com/apache/phoenix/pull/186
1. Issue - https://issues.apache.org/jira/browse/PHOENIX-3132
2. I was waiting for a response do this PR before I took on more refactoring ;)
3. I have tested in so far as the output of the two versions of the program are the same before they are shelled out in the last line. So, yes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
Sorry for the delay @btbytes, I missed your latest push where you addressed sqlline.py too. Thanks!
I think this is looking good. I left one comment for you. I will try to test this out locally today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r73030368
--- Diff: bin/sqlline-thin.py ---
@@ -18,64 +18,58 @@
# limitations under the License.
#
############################################################################
-
+from __future__ import print_function
import os
+import phoenix_utils
import subprocess
import sys
-import phoenix_utils
-import atexit
import urlparse
-global childProc
-childProc = None
-def kill_child():
- if childProc is not None:
- childProc.terminate()
- childProc.kill()
- if os.name != 'nt':
- os.system("reset")
-atexit.register(kill_child)
--- End diff --
I take half of that back. `childProc` is being used in `sqlline.py`, not in this script. I'll see if I can take care of zombie child processes in a cleaner way in both places.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on the issue:
https://github.com/apache/phoenix/pull/186
wow. it's been a month. Sorry. I'll get back to this over this weekend.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
Great! I've given you karma and assigned PHOENIX-3132 to you.
> I was waiting for a response do this PR before I took on more refactoring ;)
Hah! Smart :)
> I have tested in so far as the output of the two versions of the program are the same before they are shelled out in the last line. So, yes.
Let me clarify. I was wondering if, in your changes to be a bit more idiomatic, you had made any changes which would help us work towards automated unit tests which we could run during the normal Phoenix Maven build.
I've had it on my backburner to look at what Apache Ambari does to invoke unit tests on python code and try to borrow some of that code so that we can try to improve the quality of our python scripts and increase our confidence in future changes (such as your's).
I don't want to push that onto you, but was just curious if you had written any sort of tests as a part of these changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r73028883
--- Diff: bin/sqlline-thin.py ---
@@ -18,64 +18,58 @@
# limitations under the License.
#
############################################################################
-
+from __future__ import print_function
import os
+import phoenix_utils
import subprocess
import sys
-import phoenix_utils
-import atexit
import urlparse
-global childProc
-childProc = None
-def kill_child():
- if childProc is not None:
- childProc.terminate()
- childProc.kill()
- if os.name != 'nt':
- os.system("reset")
-atexit.register(kill_child)
--- End diff --
AFAICT, that variable was not altered anywhere after the initialisation. I grepped across the project (ugh.. globals) and did not see it changed it anywhere else either.
IMO, this is a copy-paste artifact?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r73024725
--- Diff: bin/sqlline-thin.py ---
@@ -18,64 +18,58 @@
# limitations under the License.
#
############################################################################
-
+from __future__ import print_function
import os
+import phoenix_utils
import subprocess
import sys
-import phoenix_utils
-import atexit
import urlparse
-global childProc
-childProc = None
-def kill_child():
- if childProc is not None:
- childProc.terminate()
- childProc.kill()
- if os.name != 'nt':
- os.system("reset")
-atexit.register(kill_child)
-
phoenix_utils.setPath()
-url = "localhost:8765"
-sqlfile = ""
-serialization_key = 'phoenix.queryserver.serialization'
-
-def usage_and_exit():
- sys.exit("usage: sqlline-thin.py [host[:port]] [sql_file]")
def cleanup_url(url):
- parsed = urlparse.urlparse(url)
- if parsed.scheme == "":
- url = "http://" + url
- parsed = urlparse.urlparse(url)
- if ":" not in parsed.netloc:
- url = url + ":8765"
+ p = urlparse.urlparse(url)
+ scheme = p.scheme if p.scheme else 'http'
+ netloc = p.netloc if ':' in p.netloc else p.netloc + '8765'
+ url = '{scheme}://{netloc}'.format(scheme, netloc)
--- End diff --
This is using the arguments positionally for substitution even though you provided named substitutions in the format string, right? Would it make sense to change the args to kwargs?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
> I have added what I think is cross-platform way of handling zombie? child processes.
*nux is definitely the one we care about the most :). I would expect that subprocess would already have the platform-specific stuff abstracted away for us (but maybe not)?
> However, I realise there are more than one such call in each program. I'll have atleast one more push to address this.
Cool.
> Except for a few lines, both the programs are quite similar. There is an opportunity to reuse code. But this is not an immediate concern.
Agreed. Definitely an area for continued improvements!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on the issue:
https://github.com/apache/phoenix/pull/186
I'd definitely love add unit tests by breaking up the functionality into testable parts. I have couple of concerns:
1. the use of globals, esp in the common module -- `phoenix_utils`. If we manage to isolate modules cleanly, testing will that much more meaningful
2. quite a bit of this script is to poke the environment .. which is I/O. Unit testing may not yield much. But, i'll take a fresh look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on the issue:
https://github.com/apache/phoenix/pull/186
Please test this locally. I don't have a working setup ATM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
```
% ./bin/sqlline.py
Traceback (most recent call last):
File "./bin/sqlline.py", line 139, in <module>
main()
File "./bin/sqlline.py", line 132, in main
preexec_fn=os.setsid)
File "/usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 711, in __init__
errread, errwrite)
File "/usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1343, in _execute_child
raise child_exception
OSError: [Errno 63] File name too long
```
Looks like there is an upper limit on the command that can be executed. Can you turn the heredoc-style string into an array?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r73025653
--- Diff: bin/sqlline-thin.py ---
@@ -18,64 +18,58 @@
# limitations under the License.
#
############################################################################
-
+from __future__ import print_function
import os
+import phoenix_utils
import subprocess
import sys
-import phoenix_utils
-import atexit
import urlparse
-global childProc
-childProc = None
-def kill_child():
- if childProc is not None:
- childProc.terminate()
- childProc.kill()
- if os.name != 'nt':
- os.system("reset")
-atexit.register(kill_child)
--- End diff --
It looks like you had dropped this logic completely. Is there a reason we don't need it or was it unintentional?
Ultimately, we'd want to make sure that if this script is killed/interrupted that it would also terminate the Java program it spawns. I'm not sure how subprocess would handle this now (and if it was even necessary).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on the issue:
https://github.com/apache/phoenix/pull/186
1. I have added what I think is cross-platform way of handling zombie? child processes. However, I realise there are more than one such call in each program. I'll have atleast one more push to address this.
2. Except for a few lines, both the programs are quite similar. There is an opportunity to reuse code. But this is not an immediate concern.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
Alright, I'll try to poke at this one today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline*.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r74688048
--- Diff: bin/sqlline.py ---
@@ -1,4 +1,5 @@
#!/usr/bin/env python
+"""sqlline.py"""
--- End diff --
This was to quieten PEP8 from complaining about "Missing docstring in public module". But, I'll remove it since uniform license header is probably more important.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r74614217
--- Diff: bin/sqlline.py ---
@@ -1,4 +1,5 @@
#!/usr/bin/env python
+"""sqlline.py"""
--- End diff --
Is this a convention I don't know about? From a licensing perspective, I think it would be best to make sure the ASLv2 header be the first thing after the shebang.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
No worries! Just cleaning up my inbox and thought about it again. Thank you for returning to it :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by btbytes <gi...@git.apache.org>.
Github user btbytes commented on the issue:
https://github.com/apache/phoenix/pull/186
I'm looking into this. I tried splitting the triple quoted string on space but that isn't working.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline*.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
re-ping @btbytes :). Still looking at finishing this up? It would be a very nice improvement for 4.9.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r73024052
--- Diff: bin/sqlline-thin.py ---
@@ -18,64 +18,58 @@
# limitations under the License.
#
############################################################################
-
+from __future__ import print_function
--- End diff --
I was curious what this was doing and it looks like it's some magic to make print in python3 work like it does in python2 (http://stackoverflow.com/questions/4560804/why-do-we-invoke-print-after-importing-print-function-in-python-2-6).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/phoenix/pull/186
Hi @btbytes! Thanks for the changes so far. A couple of things:
1. Have you opened a JIRA issue which captures the changes you'd like to make? We want to make sure that JIRA is the source system of record for discussing changes. You should be able to create an "Improvement" issue under https://issues.apache.org/jira/browse/PHOENIX. If you haven't already, please do so, and then let me know the URL and I can assign it to you to for credit.
2. Any thoughts about making the corresponding changes to sqlline.py that you made here to sqlline-thin.py?
3. Any thoughts about testing these changes?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #186: cleanup sqlline-thin.py using python idioms.
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/186#discussion_r73024268
--- Diff: bin/sqlline-thin.py ---
@@ -86,73 +80,83 @@ def get_serialization():
return default_serialization
return stdout
-if len(sys.argv) == 1:
- pass
-elif len(sys.argv) == 2:
- if os.path.isfile(sys.argv[1]):
- sqlfile = sys.argv[1]
- else:
+
+def main():
+ url = 'localhost:8765'
+ sqlfile = ''
+ if len(sys.argv) == 1:
+ pass
+ elif len(sys.argv) == 2:
+ if os.path.isfile(sys.argv[1]):
+ sqlfile = sys.argv[1]
+ else:
+ url = sys.argv[1]
+ elif len(sys.argv) == 3:
url = sys.argv[1]
-elif len(sys.argv) == 3:
- url = sys.argv[1]
- sqlfile = sys.argv[2]
-else:
- usage_and_exit()
-
-url = cleanup_url(url)
-
-if sqlfile != "":
- sqlfile = "--run=" + sqlfile
-
-colorSetting = "true"
-# disable color setting for windows OS
-if os.name == 'nt':
- colorSetting = "false"
-
-# HBase configuration folder path (where hbase-site.xml reside) for
-# HBase/Phoenix client side property override
-hbase_config_path = os.getenv('HBASE_CONF_DIR', phoenix_utils.current_dir)
-
-serialization = get_serialization()
-
-java_home = os.getenv('JAVA_HOME')
-
-# load hbase-env.??? to extract JAVA_HOME, HBASE_PID_DIR, HBASE_LOG_DIR
-hbase_env_path = None
-hbase_env_cmd = None
-if os.name == 'posix':
- hbase_env_path = os.path.join(hbase_config_path, 'hbase-env.sh')
- hbase_env_cmd = ['bash', '-c', 'source %s && env' % hbase_env_path]
-elif os.name == 'nt':
- hbase_env_path = os.path.join(hbase_config_path, 'hbase-env.cmd')
- hbase_env_cmd = ['cmd.exe', '/c', 'call %s & set' % hbase_env_path]
-if not hbase_env_path or not hbase_env_cmd:
- print >> sys.stderr, "hbase-env file unknown on platform %s" % os.name
- sys.exit(-1)
-
-hbase_env = {}
-if os.path.isfile(hbase_env_path):
- p = subprocess.Popen(hbase_env_cmd, stdout = subprocess.PIPE)
- for x in p.stdout:
- (k, _, v) = x.partition('=')
- hbase_env[k.strip()] = v.strip()
-
-if hbase_env.has_key('JAVA_HOME'):
- java_home = hbase_env['JAVA_HOME']
-
-if java_home:
- java = os.path.join(java_home, 'bin', 'java')
-else:
- java = 'java'
-
-java_cmd = java + ' $PHOENIX_OPTS ' + \
- ' -cp "' + phoenix_utils.hbase_conf_dir + os.pathsep + phoenix_utils.phoenix_thin_client_jar + \
- os.pathsep + phoenix_utils.hadoop_conf + os.pathsep + phoenix_utils.hadoop_classpath + '" -Dlog4j.configuration=file:' + \
- os.path.join(phoenix_utils.current_dir, "log4j.properties") + \
- " org.apache.phoenix.queryserver.client.SqllineWrapper -d org.apache.phoenix.queryserver.client.Driver " + \
- " -u \"jdbc:phoenix:thin:url=" + url + ";serialization=" + serialization + "\"" + \
- " -n none -p none --color=" + colorSetting + " --fastConnect=false --verbose=true " + \
- " --incremental=false --isolation=TRANSACTION_READ_COMMITTED " + sqlfile
-
-exitcode = subprocess.call(java_cmd, shell=True)
-sys.exit(exitcode)
+ sqlfile = sys.argv[2]
+ else:
+ sys.exit('usage: sqlline-thin.py [host[:port]] [sql_file]')
+
+ # disable color setting for windows OS
+ color_setting = 'false' if os.name == 'nt' else 'true'
+
+ # HBase configuration folder path (where hbase-site.xml reside) for
+ # HBase/Phoenix client side property override
+ hbase_config_path = os.getenv('HBASE_CONF_DIR', phoenix_utils.current_dir)
+ serialization = get_serialization()
+ java_home = os.getenv('JAVA_HOME')
+
+ # load hbase-env.??? to extract JAVA_HOME, HBASE_PID_DIR, HBASE_LOG_DIR
+ hbase_env_path = None
+ hbase_env_cmd = None
+ if os.name == 'posix':
+ hbase_env_path = os.path.join(hbase_config_path, 'hbase-env.sh')
+ hbase_env_cmd = ['bash', '-c', 'source %s && env' % hbase_env_path]
+ elif os.name == 'nt':
+ hbase_env_path = os.path.join(hbase_config_path, 'hbase-env.cmd')
+ hbase_env_cmd = ['cmd.exe', '/c', 'call %s & set' % hbase_env_path]
+
+ if not hbase_env_path:
+ print("hbase-env script unknown on platform %s" % (os.name, ),
+ file=sys.stderr)
+ sys.exit(-1)
+
+ hbase_env = {}
+ if os.path.isfile(hbase_env_path):
+ p = subprocess.Popen(hbase_env_cmd, stdout=subprocess.PIPE)
+ for x in p.stdout:
+ (k, _, v) = x.partition('=')
+ hbase_env[k.strip()] = v.strip()
+
+ if 'JAVA_HOME' in hbase_env:
+ java_home = hbase_env['JAVA_HOME']
+
+ java = os.path.join(java_home, 'bin', 'java') if java_home else 'java'
+ print ('java is ', java)
--- End diff --
Nit, please remove the print statement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---