You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/05/31 12:46:59 UTC

[GitHub] [phoenix] richardantal opened a new pull request #1243: PHOENIX-6485 clean up sqlline.py classpath

richardantal opened a new pull request #1243:
URL: https://github.com/apache/phoenix/pull/1243


   Change-Id: Iba524a971eb62c42a36eaff4ae303aab5f04e298


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1243: PHOENIX-6485 clean up sqlline.py classpath

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1243:
URL: https://github.com/apache/phoenix/pull/1243#issuecomment-851864170


   Can you do the same for the rest of the scripts ? 
   psql, pherf, etc.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1243: PHOENIX-6485 clean up sqlline.py classpath

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1243:
URL: https://github.com/apache/phoenix/pull/1243#issuecomment-852055402


   Once we are at it, can we also replace all phoenix-client JARs with phoenix-client-embedded + log4j backend jar ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1243: PHOENIX-6485 clean up sqlline.py classpath

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1243:
URL: https://github.com/apache/phoenix/pull/1243#issuecomment-886440308


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m  0s |  Docker mode activated.  |
   | -1 :x: |  patch  |   0m  3s |  https://github.com/apache/phoenix/pull/1243 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | GITHUB PR | https://github.com/apache/phoenix/pull/1243 |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1243/1/console |
   | versions | git=2.17.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1243: PHOENIX-6485 clean up sqlline.py classpath

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1243:
URL: https://github.com/apache/phoenix/pull/1243#issuecomment-851510423


   We should go further.
   
   We should remove hbase_config_path, and use hbase_conf_dir from phoenix_utils.py instead in every script.
   The same goes for hbase_conf_path, which is no longer needed.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on a change in pull request #1243: PHOENIX-6485 clean up sqlline.py classpath

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1243:
URL: https://github.com/apache/phoenix/pull/1243#discussion_r642504679



##########
File path: bin/sqlline.py
##########
@@ -108,12 +108,10 @@ def kill_child():
     colorSetting = "false"
 
 java_cmd = java + ' $PHOENIX_OPTS ' + \
-    ' -cp "' + phoenix_utils.sqlline_with_deps_jar + os.pathsep + hbase_config_path + os.pathsep + \
-    phoenix_utils.slf4j_backend_jar + os.pathsep + \
-    phoenix_utils.hbase_conf_dir + os.pathsep + phoenix_utils.phoenix_client_embedded_jar + \
-    os.pathsep + phoenix_utils.hadoop_common_jar + os.pathsep + phoenix_utils.hadoop_hdfs_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") + \
+    ' -cp "' + hbase_config_path + os.pathsep + phoenix_utils.hbase_conf_dir + os.pathsep + \
+    phoenix_utils.sqlline_with_deps_jar + os.pathsep + phoenix_utils.slf4j_backend_jar + os.pathsep + \
+    phoenix_utils.phoenix_client_embedded_jar + os.pathsep + phoenix_utils.hadoop_conf + \

Review comment:
       we should put the hadoop_conf dir after the hbase_conf




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org