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 2020/05/20 02:19:03 UTC

[GitHub] [phoenix-queryserver] infraio opened a new pull request #35: PHOENIX-5907 Remove unused part from phoenix_utils.py

infraio opened a new pull request #35:
URL: https://github.com/apache/phoenix-queryserver/pull/35


   


----------------------------------------------------------------
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-queryserver] stoty commented on a change in pull request #35: PHOENIX-5907 Remove unused part from phoenix_utils.py

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



##########
File path: bin/phoenix_utils.py
##########
@@ -113,60 +110,10 @@ def setPath():
     global phoenix_queryserver_classpath
     phoenix_queryserver_classpath = os.path.join(current_dir, "../lib/*")
 
-    global pherf_conf_path
-    pherf_conf_path = os.path.join(current_dir, "config")
-    pherf_properties_file = find("pherf.properties", pherf_conf_path)
-    if pherf_properties_file == "":
-        pherf_conf_path = os.path.join(current_dir, "..", "phoenix-pherf", "config")
-
-    global phoenix_jar_path
-    phoenix_jar_path = os.path.join(current_dir, "..", "phoenix-client", "target","*")
-
     global phoenix_client_jar
-    phoenix_client_jar = find("phoenix-*[!n]-client.jar", phoenix_jar_path)
+    phoenix_client_jar = find(PHOENIX_CLIENT_JAR_PATTERN, phoenix_class_path)
     if phoenix_client_jar == "":
         phoenix_client_jar = findFileInPathWithoutRecursion(PHOENIX_CLIENT_JAR_PATTERN, os.path.join(current_dir, ".."))
-    if phoenix_client_jar == "":
-        phoenix_client_jar = find(PHOENIX_CLIENT_JAR_PATTERN, phoenix_class_path)
-
-    global phoenix_test_jar_path
-    phoenix_test_jar_path = os.path.join(current_dir, "..", "phoenix-core", "target","*")
-
-    global hadoop_conf

Review comment:
       see comment for queryserver

##########
File path: bin/queryserver.py
##########
@@ -121,12 +119,11 @@
 #    " -XX:+UnlockCommercialFeatures -XX:+FlightRecorder -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true" + \
 
 # The command is run through subprocess so environment variables are automatically inherited
-java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + hadoop_config_path + os.pathsep + \
+java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + \

Review comment:
       I don't think that we should remove hadoop_config_path
   We need the Hadoop config files for operation, and there is no guarantee that the hbase config dir contains a copy. It seems to be deployment dependent.
   
   See https://hbase.apache.org/book.html#fully_dist




----------------------------------------------------------------
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-queryserver] asfgit closed pull request #35: PHOENIX-5907 Remove unused part from phoenix_utils.py

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #35:
URL: https://github.com/apache/phoenix-queryserver/pull/35


   


----------------------------------------------------------------
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-queryserver] infraio commented on a change in pull request #35: PHOENIX-5907 Remove unused part from phoenix_utils.py

Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #35:
URL: https://github.com/apache/phoenix-queryserver/pull/35#discussion_r434389641



##########
File path: bin/queryserver.py
##########
@@ -121,12 +119,11 @@
 #    " -XX:+UnlockCommercialFeatures -XX:+FlightRecorder -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true" + \
 
 # The command is run through subprocess so environment variables are automatically inherited
-java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + hadoop_config_path + os.pathsep + \
+java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + \

Review comment:
       But the hadoop_conf is only used to start a hbase cluster? For queryserver, it only need to know the hbase cluster, such as a hbase client, which don't need to know the hdfs config? I read queryserver's doc and start queryserver which only have hbase's config. I miss something?




----------------------------------------------------------------
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-queryserver] infraio commented on a change in pull request #35: PHOENIX-5907 Remove unused part from phoenix_utils.py

Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #35:
URL: https://github.com/apache/phoenix-queryserver/pull/35#discussion_r434435268



##########
File path: bin/queryserver.py
##########
@@ -121,12 +119,11 @@
 #    " -XX:+UnlockCommercialFeatures -XX:+FlightRecorder -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true" + \
 
 # The command is run through subprocess so environment variables are automatically inherited
-java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + hadoop_config_path + os.pathsep + \
+java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + \

Review comment:
       Got it. Thanks for explanation.




----------------------------------------------------------------
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-queryserver] stoty commented on a change in pull request #35: PHOENIX-5907 Remove unused part from phoenix_utils.py

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



##########
File path: bin/queryserver.py
##########
@@ -121,12 +119,11 @@
 #    " -XX:+UnlockCommercialFeatures -XX:+FlightRecorder -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true" + \
 
 # The command is run through subprocess so environment variables are automatically inherited
-java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + hadoop_config_path + os.pathsep + \
+java_cmd = '%(java)s -cp ' + hbase_config_path + os.pathsep + \

Review comment:
       No, the Hadoop config is also is also used by all Hadoop clients, i.e. anything that uses HDFS, MR, etc.
   
   HBase is designed to work on top of Hadoop, and uses many of its features. It needs the Hadoop config even for its clients.
   
   One example is the proxyuser settings that are defined in the Hadoop config files, and HBase and Phoenix both need to access those.
   
   The Phoenix client also uses HDFS directly (at least for loading UDF jars), so its needs to have access to all HDFS settings as well.
   
   I am sure that there are plenty of other cases where HBase (client) and Phoenix need to have access to the full Hadoop config, the above two are just the ones that have caused problems for me personally.




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