[SPARK-42342][PYTHON][CONNECT] Introduce base hierarchy to exceptions #39882

ueshin · 2023-02-04T01:32:27Z

What changes were proposed in this pull request?

Introduces base hierarchy to exceptions.

As a common hierarchy for users, base exception classes are subclasses of PySparkException.
The concrete classes for both PySpark and Spark Connect inherits the base classes that should not be exposed to users.

Why are the changes needed?

Currently exception class hierarchy is separated between PySpark and Spark Connect.

If users want to check the exception type, they need to switch the error classes based on whether they are running on PySpark or Spark Connect, but it's not ideal.

Does this PR introduce any user-facing change?

No. Users still can use the existing exception classes to check the exception type.

How was this patch tested?

Updated tests.

ueshin · 2023-02-07T14:43:26Z

cc @HyukjinKwon @zhengruifeng @itholic

dongjoon-hyun

+1, LGTM from my side. Thank you, @ueshin .

itholic

Looks pretty good. Jut left nit question for my understanding.

itholic · 2023-02-08T05:28:31Z

python/pyspark/sql/streaming/query.py

            msg = je.toString().split(": ", 1)[1]  # Drop the Java StreamingQueryException type info
            stackTrace = "\n\t at ".join(map(lambda x: x.toString(), je.getStackTrace()))
-            return StreamingQueryException(msg, stackTrace, je.getCause())
+            return CapturedStreamingQueryException(msg, stackTrace, je.getCause())


Just for clear understanding, will it raise StreamingQueryException in user space, right ?
I just want to clarify because we don't use such an alias for connect exceptions for example:

from pyspark.errors.exceptions.connect import ( AnalysisException as ConnectAnalysisException )

The name StreamingQueryException is already imported for the return type annotation, so we just need to use another name.

spark/python/pyspark/sql/streaming/query.py

Lines 24 to 27 in 673d2de

from pyspark.errors import StreamingQueryException

from pyspark.errors.exceptions.captured import (

StreamingQueryException as CapturedStreamingQueryException,

)

Oh, I missed it.
LGTM!

itholic · 2023-02-08T05:33:09Z

python/pyspark/sql/tests/connect/test_parity_column.py


 class ColumnParityTests(ColumnTestsMixin, ReusedConnectTestCase):
-    # TODO(SPARK-42017): Different error type AnalysisException vs SparkConnectAnalysisException
+    # TODO(SPARK-42017): df["bad_key"] does not raise AnalysisException


itholic · 2023-02-08T05:34:55Z

python/pyspark/sql/tests/streaming/test_streaming.py

        finally:
            sq.stop()
-        self.assertTrue(type(sq.exception()) is StreamingQueryException)
+        self.assertIsInstance(sq.exception(), StreamingQueryException)


HyukjinKwon · 2023-02-08T11:46:01Z

Merged to master and branch-3.4.

### What changes were proposed in this pull request? Introduces base hierarchy to exceptions. As a common hierarchy for users, base exception classes are subclasses of `PySparkException`. The concrete classes for both PySpark and Spark Connect inherits the base classes that should not be exposed to users. ### Why are the changes needed? Currently exception class hierarchy is separated between PySpark and Spark Connect. If users want to check the exception type, they need to switch the error classes based on whether they are running on PySpark or Spark Connect, but it's not ideal. ### Does this PR introduce _any_ user-facing change? No. Users still can use the existing exception classes to check the exception type. ### How was this patch tested? Updated tests. Closes #39882 from ueshin/issues/SPARK-42342/exceptions. Authored-by: Takuya UESHIN <ueshin@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit bd34b16) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…ise_error to call the proper test ### What changes were proposed in this pull request? This is a follow-up of #39882. Fixes `FunctionsParityTests.test_raise_error` to call the proper test. ### Why are the changes needed? `FunctionsParityTests.test_raise_error` should've called `check_raise_error`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? The fixed test. Closes #40021 from ueshin/issues/SPARK-42342/test. Authored-by: Takuya UESHIN <ueshin@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…ise_error to call the proper test ### What changes were proposed in this pull request? This is a follow-up of #39882. Fixes `FunctionsParityTests.test_raise_error` to call the proper test. ### Why are the changes needed? `FunctionsParityTests.test_raise_error` should've called `check_raise_error`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? The fixed test. Closes #40021 from ueshin/issues/SPARK-42342/test. Authored-by: Takuya UESHIN <ueshin@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 3ed1b95) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

### What changes were proposed in this pull request? Introduces base hierarchy to exceptions. As a common hierarchy for users, base exception classes are subclasses of `PySparkException`. The concrete classes for both PySpark and Spark Connect inherits the base classes that should not be exposed to users. ### Why are the changes needed? Currently exception class hierarchy is separated between PySpark and Spark Connect. If users want to check the exception type, they need to switch the error classes based on whether they are running on PySpark or Spark Connect, but it's not ideal. ### Does this PR introduce _any_ user-facing change? No. Users still can use the existing exception classes to check the exception type. ### How was this patch tested? Updated tests. Closes apache#39882 from ueshin/issues/SPARK-42342/exceptions. Authored-by: Takuya UESHIN <ueshin@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit bd34b16) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…ise_error to call the proper test ### What changes were proposed in this pull request? This is a follow-up of apache#39882. Fixes `FunctionsParityTests.test_raise_error` to call the proper test. ### Why are the changes needed? `FunctionsParityTests.test_raise_error` should've called `check_raise_error`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? The fixed test. Closes apache#40021 from ueshin/issues/SPARK-42342/test. Authored-by: Takuya UESHIN <ueshin@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 3ed1b95) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

Introduce base hierarchy to exceptions.

280cad1

github-actions bot added CONNECT CORE PYTHON SQL labels Feb 4, 2023

ueshin changed the title ~~Introduce base hierarchy to exceptions.~~ [SPARK-42342][CONNECT] Introduce base hierarchy to exceptions Feb 4, 2023

ueshin added 4 commits February 3, 2023 17:46

Rename.

608e025

Update setup.py

45fd5b6

Fix.

aa55e7d

Merge branch 'master' into issues/SPARK-42342/exceptions

45a66f1

github-actions bot added the STRUCTURED STREAMING label Feb 6, 2023

HyukjinKwon mentioned this pull request Feb 7, 2023

[SPARK-41600][SPARK-41623][SPARK-41612][CONNECT] Implement Catalog.cacheTable, isCached and uncache #39919

Closed

ueshin added 4 commits February 6, 2023 22:32

Merge branch 'master' into issues/SPARK-42342/exceptions

b77b867

Fix.

7521294

Fix.

c9df060

Fix.

673d2de

ueshin marked this pull request as ready for review February 7, 2023 07:39

dongjoon-hyun changed the title ~~[SPARK-42342][CONNECT] Introduce base hierarchy to exceptions~~ [SPARK-42342][PYTHON][CONNECT] Introduce base hierarchy to exceptions Feb 8, 2023

dongjoon-hyun approved these changes Feb 8, 2023

View reviewed changes

itholic approved these changes Feb 8, 2023

View reviewed changes

HyukjinKwon approved these changes Feb 8, 2023

View reviewed changes

zhengruifeng approved these changes Feb 8, 2023

View reviewed changes

HyukjinKwon closed this in bd34b16 Feb 8, 2023

ueshin mentioned this pull request Feb 14, 2023

[SPARK-42342][PYTHON][CONNECT][TEST] Fix FunctionsParityTests.test_raise_error to call the proper test #40021

Closed

chungyichi mentioned this pull request Jul 4, 2024

[Subtask] Fix duplicate-code Pylint rule for client-python apache/gravitino#3767

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-42342][PYTHON][CONNECT] Introduce base hierarchy to exceptions #39882

[SPARK-42342][PYTHON][CONNECT] Introduce base hierarchy to exceptions #39882

Uh oh!

ueshin commented Feb 4, 2023

Uh oh!

ueshin commented Feb 7, 2023

Uh oh!

dongjoon-hyun left a comment

Uh oh!

itholic left a comment

Uh oh!

itholic Feb 8, 2023

Uh oh!

ueshin Feb 8, 2023

Uh oh!

itholic Feb 8, 2023

Uh oh!

itholic Feb 8, 2023

Uh oh!

itholic Feb 8, 2023

Uh oh!

HyukjinKwon commented Feb 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	from pyspark.errors import StreamingQueryException
	from pyspark.errors.exceptions.captured import (
	StreamingQueryException as CapturedStreamingQueryException,
	)

[SPARK-42342][PYTHON][CONNECT] Introduce base hierarchy to exceptions #39882

[SPARK-42342][PYTHON][CONNECT] Introduce base hierarchy to exceptions #39882

Uh oh!

Conversation

ueshin commented Feb 4, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

ueshin commented Feb 7, 2023

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

itholic left a comment

Choose a reason for hiding this comment

Uh oh!

itholic Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

ueshin Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

itholic Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

itholic Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

itholic Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Feb 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants