cloud-fan commented on a change in pull request #28751:
URL: https://github.com/apache/spark/pull/28751#discussion_r436646420
##########
File path:
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SharedThriftServer.scala
##########
@@ -33,6 +33,8 @@ trait SharedThriftServer extends SharedSparkSession {
private var hiveServer2: HiveThriftServer2 = _
private var serverPort: Int = 0
+ def mode: ServerMode.Value = ServerMode.binary
Review comment:
it seems weird to have a default value in the base test trait.
##########
File path:
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SharedThriftServer.scala
##########
@@ -74,18 +86,31 @@ trait SharedThriftServer extends SharedSparkSession {
// Set the HIVE_SERVER2_THRIFT_PORT to 0, so it could randomly pick any
free port to use.
// It's much more robust than set a random port generated by ourselves
ahead
sqlContext.setConf(ConfVars.HIVE_SERVER2_THRIFT_PORT.varname, "0")
- hiveServer2 = HiveThriftServer2.startWithContext(sqlContext)
- hiveServer2.getServices.asScala.foreach {
- case t: ThriftCLIService if t.getPortNumber != 0 =>
- serverPort = t.getPortNumber
- logInfo(s"Started HiveThriftServer2: port=$serverPort,
attempt=$attempt")
- case _ =>
- }
+ // Set the HIVE_SERVER2_THRIFT_HTTP_PORT to 0, so it could randomly pick
any free port to use.
+ // It's much more robust than set a random port generated by ourselves
ahead
+ sqlContext.setConf(ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT.varname, "0")
+ sqlContext.setConf(ConfVars.HIVE_SERVER2_TRANSPORT_MODE.varname,
mode.toString)
+
+ try {
+ hiveServer2 = HiveThriftServer2.startWithContext(sqlContext)
+ hiveServer2.getServices.asScala.foreach {
+ case t: ThriftCLIService if t.getPortNumber != 0 =>
+ serverPort = t.getPortNumber
+ logInfo(s"Started HiveThriftServer2: port=$serverPort,
attempt=$attempt")
Review comment:
so we may not output this log?
##########
File path:
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SharedThriftServer.scala
##########
@@ -74,18 +86,31 @@ trait SharedThriftServer extends SharedSparkSession {
// Set the HIVE_SERVER2_THRIFT_PORT to 0, so it could randomly pick any
free port to use.
// It's much more robust than set a random port generated by ourselves
ahead
sqlContext.setConf(ConfVars.HIVE_SERVER2_THRIFT_PORT.varname, "0")
- hiveServer2 = HiveThriftServer2.startWithContext(sqlContext)
- hiveServer2.getServices.asScala.foreach {
- case t: ThriftCLIService if t.getPortNumber != 0 =>
- serverPort = t.getPortNumber
- logInfo(s"Started HiveThriftServer2: port=$serverPort,
attempt=$attempt")
- case _ =>
- }
+ // Set the HIVE_SERVER2_THRIFT_HTTP_PORT to 0, so it could randomly pick
any free port to use.
+ // It's much more robust than set a random port generated by ourselves
ahead
+ sqlContext.setConf(ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT.varname, "0")
+ sqlContext.setConf(ConfVars.HIVE_SERVER2_TRANSPORT_MODE.varname,
mode.toString)
+
+ try {
+ hiveServer2 = HiveThriftServer2.startWithContext(sqlContext)
+ hiveServer2.getServices.asScala.foreach {
+ case t: ThriftCLIService if t.getPortNumber != 0 =>
+ serverPort = t.getPortNumber
+ logInfo(s"Started HiveThriftServer2: port=$serverPort,
attempt=$attempt")
Review comment:
how does this patch fix it? It seems you just added a try-catch?
##########
File path:
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SharedThriftServer.scala
##########
@@ -74,18 +86,31 @@ trait SharedThriftServer extends SharedSparkSession {
// Set the HIVE_SERVER2_THRIFT_PORT to 0, so it could randomly pick any
free port to use.
// It's much more robust than set a random port generated by ourselves
ahead
sqlContext.setConf(ConfVars.HIVE_SERVER2_THRIFT_PORT.varname, "0")
- hiveServer2 = HiveThriftServer2.startWithContext(sqlContext)
- hiveServer2.getServices.asScala.foreach {
- case t: ThriftCLIService if t.getPortNumber != 0 =>
- serverPort = t.getPortNumber
- logInfo(s"Started HiveThriftServer2: port=$serverPort,
attempt=$attempt")
- case _ =>
- }
+ // Set the HIVE_SERVER2_THRIFT_HTTP_PORT to 0, so it could randomly pick
any free port to use.
+ // It's much more robust than set a random port generated by ourselves
ahead
+ sqlContext.setConf(ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT.varname, "0")
+ sqlContext.setConf(ConfVars.HIVE_SERVER2_TRANSPORT_MODE.varname,
mode.toString)
+
+ try {
+ hiveServer2 = HiveThriftServer2.startWithContext(sqlContext)
+ hiveServer2.getServices.asScala.foreach {
+ case t: ThriftCLIService if t.getPortNumber != 0 =>
+ serverPort = t.getPortNumber
+ logInfo(s"Started HiveThriftServer2: port=$serverPort,
attempt=$attempt")
Review comment:
ah I see!
##########
File path:
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SharedThriftServer.scala
##########
@@ -53,11 +55,21 @@ trait SharedThriftServer extends SharedSparkSession {
}
}
+ protected def jdbcUri: String = if (mode == ServerMode.http) {
+ s"""jdbc:hive2://localhost:$serverPort/
+ |default;
+ |transportMode=http;
+ |httpPath=cliservice
+ """.stripMargin.split("\n").mkString.trim
+ } else {
+ s"""jdbc:hive2://localhost:$serverPort"""
+ }
Review comment:
I think the existing format is correct.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
