You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "amaliujia (via GitHub)" <gi...@apache.org> on 2023/05/09 15:45:10 UTC

[GitHub] [spark] amaliujia commented on a diff in pull request #41097: [SPARK-43418][CONNECT] Add SparkSession.Builder.getOrCreate

amaliujia commented on code in PR #41097:
URL: https://github.com/apache/spark/pull/41097#discussion_r1188803489


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/SparkSession.scala:
##########
@@ -586,23 +610,56 @@ object SparkSession extends Logging {
   }
 
   class Builder() extends Logging {
-    private var _client: SparkConnectClient = _
+    private val builder = SparkConnectClient.builder()
+    private var client: SparkConnectClient = _
 
     def remote(connectionString: String): Builder = {
-      client(SparkConnectClient.builder().connectionString(connectionString).build())
+      builder.connectionString(connectionString)
       this
     }
 
     private[sql] def client(client: SparkConnectClient): Builder = {
-      _client = client
+      this.client = client
       this
     }
 
-    def build(): SparkSession = {
-      if (_client == null) {
-        _client = SparkConnectClient.builder().build()
+    private def tryCreateSessionFromClient(): Option[SparkSession] = {
+      if (client != null) {
+        Option(new SparkSession(client, cleaner, planIdGenerator))
+      } else {
+        None
       }
-      new SparkSession(_client, cleaner, planIdGenerator)
+    }
+
+    /**
+     * Build the [[SparkSession]].
+     *
+     * This will always return a newly created session.
+     */
+    @deprecated(message = "Please use create() instead.", since = "3.5.0")
+    def build(): SparkSession = create()
+
+    /**
+     * Create a new [[SparkSession]].
+     *
+     * This will always return a newly created session.
+     *
+     * @since 3.5.0
+     */
+    def create(): SparkSession = {
+      tryCreateSessionFromClient().getOrElse(SparkSession.this.create(builder.configuration))

Review Comment:
   How about the newSession API. Should we follow the same semantics defined here?



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org