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/08/07 04:50:40 UTC
[GitHub] [spark] HyukjinKwon opened a new pull request, #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
HyukjinKwon opened a new pull request, #42371:
URL: https://github.com/apache/spark/pull/42371
### What changes were proposed in this pull request?
This PR proposes to (mostly) refactor all the internal workarounds to get the active session correctly.
There are few things to note:
- _PySpark with Spark Connect does not already support the hierarchy of active sessions_. With pinned thread mode (enabled by default), PySpark does map each Python thread to JVM thread, but the thread creation happens within gateway server, that does not respect the thread hierarchy. Therefore, this PR follows the exactly same behaviour.
- New thread will not have an active thread by default.
- Other behaviours are same as PySpark without Connect, see also https://github.com/apache/spark/pull/42367
- Since I am here, I piggiyback few documentation changes. We missed document `SparkSession.readStream`, `SparkSession.streams`, `SparkSession.udtf`, `SparkSession.conf` and `SparkSession.version` in Spark Connect.
- The changes here are mostly refactoring that reuses existing unittests while I expose two methods:
- `SparkSession.getActiveSession` (only for Spark Connect)
- `SparkSession.active` (for both in PySpark)
### Why are the changes needed?
For Spark Connect users to be able to play with active and default sessions in Python.
### Does this PR introduce _any_ user-facing change?
Yes, it adds new API:
- `SparkSession.getActiveSession` (only for Spark Connect)
- `SparkSession.active` (for both in PySpark)
### How was this patch tested?
Existing unittests should cover all.
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #42371:
URL: https://github.com/apache/spark/pull/42371#issuecomment-1667183647
cc @hvanhovell @ueshin @allisonwang-db @rangadi @WeichenXu123 @xinrong-meng @zhengruifeng @itholic. I happened to touch here and there - would be great if you guys find some time to take a look.
--
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] ueshin commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286422331
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
+ # Reference to the root SparkSession
+ _default_session: Optional["SparkSession"] = None
Review Comment:
The type should be `ClassVar[Optional["SparkSession"]]`?
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
+ # Reference to the root SparkSession
+ _default_session: Optional["SparkSession"] = None
+ _lock = RLock()
Review Comment:
ditto, `ClassVar[RLock]]`?
##########
python/pyspark/sql/session.py:
##########
@@ -805,6 +847,9 @@ def udtf(self) -> "UDTFRegistration":
.. versionadded:: 3.5.0
+ .. versionchanged:: 3.4.0
+ Supports Spark Connect.
Review Comment:
This is introduced in `3.5.0`.
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
Review Comment:
Need the type hint for this?
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1285421989
##########
python/pyspark/errors/error_classes.py:
##########
@@ -622,6 +622,11 @@
"No active Spark session found. Please create a new Spark session before running the code."
]
},
+ "NO_ACTIVE_OR_DEFAULT_SESSION" : {
Review Comment:
How is it different from `NOT_STR`, `NOT_STR_OR_LIST_OF_RDD` and `NOT_STR_OR_STRUCT` cases?
--
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] itholic commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1285406934
##########
python/pyspark/errors/error_classes.py:
##########
@@ -622,6 +622,11 @@
"No active Spark session found. Please create a new Spark session before running the code."
]
},
+ "NO_ACTIVE_OR_DEFAULT_SESSION" : {
Review Comment:
We already have `NO_ACTIVE_SESSION`. Maybe we can update the existing error class?
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #42371:
URL: https://github.com/apache/spark/pull/42371#issuecomment-1668799750
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] HyukjinKwon commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1285856139
##########
python/pyspark/ml/connect/tuning.py:
##########
@@ -178,11 +178,12 @@ def _parallelFitTasks(
def get_single_task(index: int, param_map: Any) -> Callable[[], Tuple[int, float]]:
def single_task() -> Tuple[int, float]:
- # Active session is thread-local variable, in background thread the active session
- # is not set, the following line sets it as the main thread active session.
- active_session._jvm.SparkSession.setActiveSession( # type: ignore[union-attr]
- active_session._jsparkSession # type: ignore[union-attr]
- )
+ if not is_remote():
Review Comment:
cc @WeichenXu123 FYI the related tests were `pyspark.ml.tests.connect.test_connect_tuning.CrossValidatorTestsOnConnect`.
--
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] ueshin commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286461234
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
Review Comment:
I guess `ClassVar[threading.local]`?
But ok, should be fine.
--
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] itholic commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286494110
##########
python/pyspark/errors/error_classes.py:
##########
@@ -622,6 +622,11 @@
"No active Spark session found. Please create a new Spark session before running the code."
]
},
+ "NO_ACTIVE_OR_DEFAULT_SESSION" : {
Review Comment:
They are type related error classes, so we separated them to inform users all the exact possible types for each cases.
So I thought we can reuse the existing error class for this case, we can also separate for this case on my second thought, nvm.
--
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] ueshin commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286461234
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
Review Comment:
I guess `ClassVar[threading.local]`?
But ok, should be fine.
I just wanted to have `mypy` check if instance methods don't update the variable.
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286324742
##########
python/pyspark/sql/connect/session.py:
##########
@@ -236,6 +237,38 @@ def __init__(self, connection: Union[str, ChannelBuilder], userId: Optional[str]
self._client = SparkConnectClient(connection=connection, user_id=userId)
self._session_id = self._client._session_id
+ @classmethod
+ def _set_default_and_active_session(cls, session: "SparkSession") -> None:
+ """
+ Set the (global) default :class:`SparkSession`, and (thread-local)
+ active :class:`SparkSession` when they are not set yet.
+ """
+ with cls._lock:
+ if cls._default_session is None:
+ cls._default_session = session
+ if getattr(cls._active_session, "session", None) is None:
+ cls._active_session.session = session
+
+ @classmethod
+ def getActiveSession(cls) -> Optional["SparkSession"]:
Review Comment:
Yup, it's same as the non Spark Connect one.
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286324742
##########
python/pyspark/sql/connect/session.py:
##########
@@ -236,6 +237,38 @@ def __init__(self, connection: Union[str, ChannelBuilder], userId: Optional[str]
self._client = SparkConnectClient(connection=connection, user_id=userId)
self._session_id = self._client._session_id
+ @classmethod
+ def _set_default_and_active_session(cls, session: "SparkSession") -> None:
+ """
+ Set the (global) default :class:`SparkSession`, and (thread-local)
+ active :class:`SparkSession` when they are not set yet.
+ """
+ with cls._lock:
+ if cls._default_session is None:
+ cls._default_session = session
+ if getattr(cls._active_session, "session", None) is None:
+ cls._active_session.session = session
+
+ @classmethod
+ def getActiveSession(cls) -> Optional["SparkSession"]:
Review Comment:
yup
--
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] ueshin commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286413587
##########
python/pyspark/sql/connect/session.py:
##########
@@ -628,20 +664,18 @@ def is_stopped(self) -> bool:
"""
return self.client.is_closed
- @classmethod
- def getActiveSession(cls) -> Any:
- raise PySparkNotImplementedError(
- error_class="NOT_IMPLEMENTED", message_parameters={"feature": "getActiveSession()"}
- )
-
@property
def conf(self) -> RuntimeConf:
return RuntimeConf(self.client)
+ conf.__doc__ = PySparkSession.conf.__doc__
Review Comment:
Good catch!
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286447366
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
Review Comment:
I am actually not sure how to type this .. it's a private var so should be fine.
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #42371:
URL: https://github.com/apache/spark/pull/42371#issuecomment-1667844533
cc @cdkrot @grundprinzip too
--
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] allisonwang-db commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "allisonwang-db (via GitHub)" <gi...@apache.org>.
allisonwang-db commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286270191
##########
python/pyspark/sql/connect/session.py:
##########
@@ -236,6 +237,38 @@ def __init__(self, connection: Union[str, ChannelBuilder], userId: Optional[str]
self._client = SparkConnectClient(connection=connection, user_id=userId)
self._session_id = self._client._session_id
+ @classmethod
+ def _set_default_and_active_session(cls, session: "SparkSession") -> None:
+ """
+ Set the (global) default :class:`SparkSession`, and (thread-local)
+ active :class:`SparkSession` when they are not set yet.
+ """
+ with cls._lock:
+ if cls._default_session is None:
+ cls._default_session = session
+ if getattr(cls._active_session, "session", None) is None:
+ cls._active_session.session = session
+
+ @classmethod
+ def getActiveSession(cls) -> Optional["SparkSession"]:
Review Comment:
Is this also a public API? I found it a bit confusing that this `.getActiveSession()` returns the active session but `.active()` returns the active or the default session (and throws an exception if no active session). Is the API the same as non Spark Connect one?
--
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] itholic commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286494110
##########
python/pyspark/errors/error_classes.py:
##########
@@ -622,6 +622,11 @@
"No active Spark session found. Please create a new Spark session before running the code."
]
},
+ "NO_ACTIVE_OR_DEFAULT_SESSION" : {
Review Comment:
They are type related error classes, so we separated them to inform users all the exact possible types for each cases.
On my second thought, we can also separate for this case, nvm.
--
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] itholic commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1285406934
##########
python/pyspark/errors/error_classes.py:
##########
@@ -622,6 +622,11 @@
"No active Spark session found. Please create a new Spark session before running the code."
]
},
+ "NO_ACTIVE_OR_DEFAULT_SESSION" : {
Review Comment:
Maybe we can update the `NO_ACTIVE_SESSION` instead of adding new one?
--
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] ueshin commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1286461234
##########
python/pyspark/sql/connect/session.py:
##########
@@ -93,14 +94,13 @@
from pyspark.sql.connect.udtf import UDTFRegistration
-# `_active_spark_session` stores the active spark connect session created by
-# `SparkSession.builder.getOrCreate`. It is used by ML code.
-# If sessions are created with `SparkSession.builder.create`, it stores
-# The last created session
-_active_spark_session = None
-
-
class SparkSession:
+ # The active SparkSession for the current thread
+ _active_session = threading.local()
Review Comment:
I guess `ClassVar[threading.local]`?
But ok, should be fine.
I just wanted to let `mypy` check if instance methods don't update the variable.
--
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 #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
URL: https://github.com/apache/spark/pull/42371
--
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] itholic commented on a diff in pull request #42371: [SPARK-44694][PYTHON][CONNECT] Refactor active sessions and expose them as an API
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42371:
URL: https://github.com/apache/spark/pull/42371#discussion_r1285406934
##########
python/pyspark/errors/error_classes.py:
##########
@@ -622,6 +622,11 @@
"No active Spark session found. Please create a new Spark session before running the code."
]
},
+ "NO_ACTIVE_OR_DEFAULT_SESSION" : {
Review Comment:
We already have `NO_ACTIVE_SESSION`. Maybe we can update the existing error class instead of adding new one?
--
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