You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HyukjinKwon (via GitHub)" <gi...@apache.org> on 2023/07/25 07:32:39 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request, #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API

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

   ### What changes were proposed in this pull request?
   
   This PR is a kind of a followup of https://github.com/apache/spark/pull/42120. This addresses my own comment at https://github.com/apache/spark/pull/42120#discussion_r1272933655 by adding the support of `InheritableThread` with Spark Connect.
   
   ### Why are the changes needed?
   
   In order to allow the thread local hierarchy supported in Scala side. 
   
   ### Does this PR introduce _any_ user-facing change?
   
   The main PR is not released out yet so it doesn't have user-facing change.
   
   ### How was this patch tested?
   
   Unittests were added.


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


[GitHub] [spark] HyukjinKwon commented on pull request #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API

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

   cc @zhengruifeng and @WeichenXu123 I need your look 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


[GitHub] [spark] HyukjinKwon commented on pull request #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API

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

   All tests passed. The failed test is pip packaging test that happens after running all unittests. I will fix them separately at https://github.com/apache/spark/pull/42159.
   
   Merged to master and branch-3.5.


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


[GitHub] [spark] zhengruifeng commented on pull request #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API

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

   LGTM, but I think we need @WeichenXu123 's approval for this PR


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


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API

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


##########
python/pyspark/util.py:
##########
@@ -348,56 +373,87 @@ def wrapped(*args: Any, **kwargs: Any) -> Any:
 
 class InheritableThread(threading.Thread):
     """
-    Thread that is recommended to be used in PySpark instead of :class:`threading.Thread`
-    when the pinned thread mode is enabled. The usage of this class is exactly same as
-    :class:`threading.Thread` but correctly inherits the inheritable properties specific
-    to JVM thread such as ``InheritableThreadLocal``.
-
-    Also, note that pinned thread mode does not close the connection from Python

Review Comment:
   This is already fixed in Py4J, https://github.com/py4j/py4j/pull/471



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


[GitHub] [spark] HyukjinKwon closed pull request #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #42149: [SPARK-44509][CONNECT][FOLLOW-UP] Add inheritable support in new job cancellation API
URL: https://github.com/apache/spark/pull/42149


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