You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2024/10/31 11:08:51 UTC

[PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

panbingkun opened a new pull request, #48720:
URL: https://github.com/apache/spark/pull/48720

   ### What changes were proposed in this pull request?
   The pr aims to print the `server's webUrl` when the connect client starts.
   
   
   ### Why are the changes needed?
   Now suppose a scenario where an end-user starts a connect client locally and executes some SQL. At that time, `he/her` wants to have a deeper understanding of the SQL execution status, but there is currently no way. Therefore, I propose that when the connect client starts, the `spark web URL` of the server should be displayed to the end-user for observation and monitoring.
   
   - Before
   
   
   - After
     <img width="933" alt="image" src="https://github.com/user-attachments/assets/03b66d80-6b50-45ae-b1ba-bc9b7fef66c8">
   
   
   ### Does this PR introduce _any_ user-facing change?
   Yes, the connect client end-user can obtain the `spark web URL` of the connect server upon startup.
   
   
   ### How was this patch tested?
   Manually check.
   
   
   ### Was this patch authored or co-authored using generative AI tooling?
   No.
   


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


Re: [PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2453932234

   > Generally, a useful change. Can you please update the Python client as well.
   
   Done.


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2460870432

   A high-level question: what about introducing a general method like `getProperty(key: String): String`, then we can reuse it to expose more similar properties?


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2716707617

   > +1 for the idea. Could you rebase this once more, @panbingkun ?
   
   Sorry, I have been struggling with a new job recently,
   and it has been updated!
   Thanks.


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "the-sakthi (via GitHub)" <gi...@apache.org>.
the-sakthi commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2610560422

   LGTM


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48720:
URL: https://github.com/apache/spark/pull/48720#discussion_r1831954159


##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -3253,3 +3253,18 @@ private[spark] class CircularBuffer(sizeInBytes: Int = 10240) extends java.io.Ou
     new String(nonCircularBuffer, StandardCharsets.UTF_8)
   }
 }
+
+object WebUrlUtils {
+  def getSparkWebUrl(context: SparkContext): Option[String] = {

Review Comment:
   That’s a good question. It was originally extracted from `SparkILoop` as a `common` method
   https://github.com/apache/spark/blob/f2d39b958deb9bae2cc39410144fe263dc96a0ef/repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala#L47-L59
   
   I think it depends on the following configuration:
   https://spark.apache.org/docs/3.5.2/configuration.html
   <img width="1105" alt="image" src="https://github.com/user-attachments/assets/cb1a1922-ec33-4c63-b359-60061897c798">
   
   However `sc.uiWebUrl` depends on the following configuration:
   https://github.com/apache/spark/blob/f2d39b958deb9bae2cc39410144fe263dc96a0ef/core/src/main/scala/org/apache/spark/ui/WebUI.scala#L175
   https://github.com/apache/spark/blob/f2d39b958deb9bae2cc39410144fe263dc96a0ef/core/src/main/scala/org/apache/spark/ui/WebUI.scala#L58
   https://github.com/apache/spark/blob/f2d39b958deb9bae2cc39410144fe263dc96a0ef/core/src/main/scala/org/apache/spark/internal/config/package.scala#L1161-L1165



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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #48720: [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl
URL: https://github.com/apache/spark/pull/48720


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48720:
URL: https://github.com/apache/spark/pull/48720#discussion_r1831957215


##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -3253,3 +3253,18 @@ private[spark] class CircularBuffer(sizeInBytes: Int = 10240) extends java.io.Ou
     new String(nonCircularBuffer, StandardCharsets.UTF_8)
   }
 }
+
+object WebUrlUtils {
+  def getSparkWebUrl(context: SparkContext): Option[String] = {

Review Comment:
   I think we should align this logic in other places where `sc.uiWebUrl` is used, but of course, this is another PR. What I want to do next is. eg:
   - spark-sql
   https://github.com/apache/spark/blob/f2d39b958deb9bae2cc39410144fe263dc96a0ef/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala#L419



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


Re: [PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48720:
URL: https://github.com/apache/spark/pull/48720#discussion_r1827253184


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/application/ConnectRepl.scala:
##########
@@ -102,8 +102,11 @@ Spark session available as 'spark'.
         |""".stripMargin
     // Please note that we make ammonite generate classes instead of objects.
     // Classes tend to have superior serialization behavior when using UDFs.
+    val webUrl = spark.webUrl
+      .map(url => s"\nSpark connect server Context Web UI is available at $url.")

Review Comment:
   Updated



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


Re: [PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2449603892

   cc @zhengruifeng @HyukjinKwon @LuciferYang 


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48720:
URL: https://github.com/apache/spark/pull/48720#discussion_r1831618548


##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -3253,3 +3253,18 @@ private[spark] class CircularBuffer(sizeInBytes: Int = 10240) extends java.io.Ou
     new String(nonCircularBuffer, StandardCharsets.UTF_8)
   }
 }
+
+object WebUrlUtils {
+  def getSparkWebUrl(context: SparkContext): Option[String] = {

Review Comment:
   how is this different from `sc.uiWebUrl`?



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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2461174511

   > A high-level question: what about introducing a general method like `getProperty(key: String): String`, then we can reuse it to expose more similar properties?
   
   Sure, allow me to do it.


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


Re: [PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2451234166

   cc @hvanhovell @grundprinzip 


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


Re: [PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #48720:
URL: https://github.com/apache/spark/pull/48720#discussion_r1825674459


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/application/ConnectRepl.scala:
##########
@@ -102,8 +102,11 @@ Spark session available as 'spark'.
         |""".stripMargin
     // Please note that we make ammonite generate classes instead of objects.
     // Classes tend to have superior serialization behavior when using UDFs.
+    val webUrl = spark.webUrl
+      .map(url => s"\nSpark connect server Context Web UI is available at $url.")

Review Comment:
   ```suggestion
         .map(url => s"\nSpark Connect server context Web UI is available at $url.")
   ```



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


Re: [PR] [SPARK-50188][CONNECT] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2451743711

   > Generally, a useful change. Can you please update the Python client as well.
   
   Allow me to do it, thank you very much for the review! ❤️


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2458512509

   > Let's fix up the conflict
   
   Thanks! It has been resolved. Let's waiting for CI.


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


Re: [PR] [SPARK-50188][CONNECT][PYTHON] When the connect client starts, print the server's webUrl [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48720:
URL: https://github.com/apache/spark/pull/48720#issuecomment-2993191826

   We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!


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