cloud-fan commented on code in PR #58396:
URL: https://github.com/apache/spark/pull/58396#discussion_r4400296398
##########
python/pyspark/sql/connect/client/core.py:
##########
@@ -2604,11 +2606,9 @@ def _delete_ml_cache(self, cache_ids: List[str],
evict_only: bool = False) -> Li
return []
def _on_exit(self) -> None:
- # If the client has already been explicitly closed, skip all cleanup
RPCs.
- # The server-side resources were released by close(); reissuing them
here
- # is wasted work and, if the server has since become unreachable, can
- # block process exit on the gRPC call.
- if self._closed:
+ # Skip cleanup RPCs if the session is already closed or released, or
was never
+ # established. The server-side session ID may be "" rather than None.
+ if self._closed or self._session_released or not
self._server_session_id:
Review Comment:
Thanks, verified on the latest commit. `_on_exit()` now returns early only
for a closed or released client, and only `_cleanup_ml_cache()` depends on an
observed server-side session ID. So with SPARK_CONNECT_RELEASE_SESSION_ON_EXIT
enabled, artifact-only and cloned clients are released and closed at exit
again, and the updated no-ID test asserts it.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4320692344","thread_id":"inline:4320692344","verdict_sha256":"c04c7e7fe42face0483acaa84f402353b450bf00b5e7c6f39b0b55ac74022b47"}
-->
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
