You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "gaogaotiantian (via GitHub)" <gi...@apache.org> on 2026/03/30 22:48:36 UTC
[PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
gaogaotiantian opened a new pull request, #55100:
URL: https://github.com/apache/spark/pull/55100
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Include Java side error classes in Python so python can generate consistent error messages as Java with the same error class + message parameters.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Currently we have two separate `error-conditions.json` files, for Python and Java. I think it's historical reason that we can't easily load Java's json on Python so we created a Python specific one. The issue is that two files are unsynced. It's possible that we want to raise the same error on both Python and Java, and we either need to create a new error class on Python json file or forget to do that and break message generation.
Ideally we should have a single source of truth, which is achievable now by merging every existing Python error classes into Java one. But there could be unexpected consequences and it's difficult to split them back once it's merged. Moreover, the Python file contains some python-specific error classes which may not fit into the java file.
As a compromise, we keep the Python file and load the Java file from Python. We keep the current structure and make it more flexible. In the future, if we have a clear plan of which way we should go, we can always eliminate the Python json file.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as new features, bug fixes, or other behavior changes. Documentation-only updates are not considered user-facing changes.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
A new test is added to test Python can raise Java-specific error class.
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
No.
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3037447071
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
Hm. Actually I wonder if we just add a linter to sync JSON and Python error classes ....
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3041994760
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
What I prefer is just to make it easier to maintain. e.g., I got some questions about this error class files time to time still, and why are they different.
Another approach might be just add a check if the error class exists in Exception class itself that is only enabled when testing (e.g., SPARK_TESTING environment variable with an assert).
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "devin-petersohn (via GitHub)" <gi...@apache.org>.
devin-petersohn commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3022027354
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
Will this impact python Spark Connect client-only installations? They would not have the jars installed.
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3023908396
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
Good question. So for now, it won't generate any error. The connect client would miss the java side error classes. As long as connect client only uses python error classes it 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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3042097368
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
> why are they different
I think I mentioned this in the long comment I wrote above :) Basically here will be some error classes that are Python specific (like connect client errors), and it would be weird to put it to Java source code. Java also doesn't need to know this.
We can have all kind of validations for this, for now we only need to decide how to organize these error classes.
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3041764110
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
We can add any linter that we want, but we need to first figure out how we should organize our error classes.
We have two questions:
* how many "real" JSON files do we want (source of truth)
* how many copies do we want (to make things like packaging easier)
We can have linter to enforce our ideas so we don't have to worry about how to make it work at this point, just what we want.
For this current PR, we have 2 source of truth (JAVA + Python) and no copy. Python has Python + JAVA and JAVA has JAVA only. The good thing about this is:
* Python always has all the error classes
* No sync needed
* Python can have its dedicated error classes (client error for example)
We can have a single error class file put in JAVA source code. We probably have to copy it to python because we need to have that accessible to connect clients (we can actually read it from classic though).
The question is - we probably do have error classes that are specific to Python code, putting those in the JAVA json file is a bit weird to me. That's why I chose this structure.
This indeed introduced the problem of connect client not able to get the JAVA error class - which is actually our current behavior (before this PR). Python does not understand JAVA error class at all before this PR and we are fine. If we want the client to have all the error classes, we can copy the JAVA side json to python and enforce the sync checking in CI.
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #55100:
URL: https://github.com/apache/spark/pull/55100#discussion_r3037446624
##########
python/pyspark/errors/error_classes.py:
##########
@@ -15,14 +15,42 @@
# limitations under the License.
#
+import glob
import json
import importlib.resources
+import os
+import zipfile
+from pyspark.find_spark_home import _find_spark_home
# Note: Though we call them "error classes" here, the proper name is "error conditions",
# hence why the name of the JSON file is different.
# For more information, please see: https://issues.apache.org/jira/browse/SPARK-46810
# This discrepancy will be resolved as part of: https://issues.apache.org/jira/browse/SPARK-47429
-ERROR_CLASSES_JSON = (
- importlib.resources.files("pyspark.errors").joinpath("error-conditions.json").read_text()
-)
-ERROR_CLASSES_MAP = json.loads(ERROR_CLASSES_JSON)
+
+
+def get_error_classes() -> dict[str, dict]:
Review Comment:
I wonder if we can add a simple validation for that. Otherwise, the error might fail with a weird error, e.g., if error code in JVM is mistakenly used in Spark Connect code.
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #55100:
URL: https://github.com/apache/spark/pull/55100#issuecomment-4986766858
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.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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
Re: [PR] [SPARK-56295][PYTHON] Add Java error classes to Python side [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #55100: [SPARK-56295][PYTHON] Add Java error classes to Python side
URL: https://github.com/apache/spark/pull/55100
--
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