You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "itholic (via GitHub)" <gi...@apache.org> on 2023/04/25 07:54:24 UTC
[GitHub] [spark] itholic opened a new pull request, #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
itholic opened a new pull request, #40939:
URL: https://github.com/apache/spark/pull/40939
### What changes were proposed in this pull request?
This PR proposes to migrate Spark Connect Window errors into error class
### Why are the changes needed?
To improve PySpark error usability.
### Does this PR introduce _any_ user-facing change?
No API changes.
### How was this patch tested?
The existing CI should pass.
--
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 #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #40939:
URL: https://github.com/apache/spark/pull/40939#discussion_r1176535987
##########
python/pyspark/errors/error_classes.py:
##########
@@ -224,6 +224,11 @@
"Argument `<arg_name>` should be a Column, int or str, got <arg_type>."
]
},
+ "NOT_COLUMN_OR_LIST_OR_STR" : {
Review Comment:
Yes, we can either merge them into single error class as you mentioned, or create a representative main error class and add related type errors as sub-error classes under main error class what we do in JVM side. Each way has its pros and cons, so we plan to proceed with the initial migration in the current form first, and then gather opinions from ppl through decision docs or something before making changes all at once.
I already created SPARK-43020 for address this issue, so let me do the refactoring once the basic migration is complete.
--
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] zhengruifeng commented on a diff in pull request #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40939:
URL: https://github.com/apache/spark/pull/40939#discussion_r1176376780
##########
python/pyspark/errors/error_classes.py:
##########
@@ -224,6 +224,11 @@
"Argument `<arg_name>` should be a Column, int or str, got <arg_type>."
]
},
+ "NOT_COLUMN_OR_LIST_OR_STR" : {
Review Comment:
I see there are a lot of `NOT_COLUMN_XXX` error classes, I am wondering is it possible to combine them into single 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] zhengruifeng closed pull request #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
URL: https://github.com/apache/spark/pull/40939
--
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 #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #40939:
URL: https://github.com/apache/spark/pull/40939#discussion_r1176535987
##########
python/pyspark/errors/error_classes.py:
##########
@@ -224,6 +224,11 @@
"Argument `<arg_name>` should be a Column, int or str, got <arg_type>."
]
},
+ "NOT_COLUMN_OR_LIST_OR_STR" : {
Review Comment:
Yes, we can either merge them into single error class as you mentioned, or create a representative main error class and add related type errors as sub-error classes under main error class what we do in JVM side.
I already created SPARK-43020 for address this issue, so let me do the refactoring once the basic migration is complete.
--
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] zhengruifeng commented on pull request #40939: [SPARK-43276][CONNECT][PYTHON] Migrate Spark Connect Window errors into error class
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40939:
URL: https://github.com/apache/spark/pull/40939#issuecomment-1522737555
merged to master
--
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