[SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions#38728
[SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions#38728grundprinzip wants to merge 8 commits intoapache:masterfrom
Conversation
|
Can one of the admins verify this patch? |
| private val message: String = "", | ||
| private val cause: Throwable = None.orNull) | ||
| extends Exception(message, cause) | ||
| extends SparkException( |
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
Outdated
Show resolved
Hide resolved
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
Outdated
Show resolved
Hide resolved
|
@itholic Finally got around addressing your comments, please have another look. Thanks! |
...r/connect/src/test/scala/org/apache/spark/sql/connect/planner/SparkConnectPlannerSuite.scala
Outdated
Show resolved
Hide resolved
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
Outdated
Show resolved
Hide resolved
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
Outdated
Show resolved
Hide resolved
|
I will add tests for the missing cases. |
grundprinzip
left a comment
grundprinzip
left a comment
There was a problem hiding this comment.
Addressed the comments, added tests.
| "Invalid Set operation: <type> does not support <param>." | ||
| ] | ||
| }, | ||
| "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : { |
There was a problem hiding this comment.
It's the underlying operator.
def fillna(
self,
value: Union["LiteralType", Dict[str, "LiteralType"]],
subset: Optional[Union[str, Tuple[str, ...], List[str]]] = None,
) -> "DataFrame":
"""Replace null values, alias for ``na.fill()``.
:func:`DataFrame.fillna` and :func:`DataFrameNaFunctions.fill` are aliases of each other.
There was a problem hiding this comment.
Uhm, I'm now no smarter. The real question is: Will users know what to do with this error?
|
General comment. You use CONNECT as error class, everything else is a sub error class. Many of these are INVALID_PLAN. |
|
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. |
6 participants
What changes were proposed in this pull request?
Migrate existing custom exceptions in Spark Connect to use the proper Spark exceptions.
Why are the changes needed?
Consistency
Does this PR introduce any user-facing change?
No
How was this patch tested?
existing UT